search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.03k stars 11.96k forks source link

Usage of isKnownWindowsMSVCEnvironment in LLVM CodeGen causes issues for IR without an environment #41836

Open rnk opened 5 years ago

rnk commented 5 years ago
Bugzilla Link 42491
Version trunk
OS Windows NT
CC @pogo59

Extended Description

Most tools (clang, llc, etc) normalize triples passed to them on the command line. This mainly has the effect of defaulting in the "msvc" environment when a triple like "x86_64-pc-win32" is passed, to produce "x86_64-pc-windows-msvc". However, llc doesn't normalize triples from IR files, so the behavior between an input with a triple and using the same input on the command line differs. We use this triple predicate in a few places:

$ git grep isKnownWindowsMSVCEnvironment ../llvm ../llvm/include/llvm/ADT/Triple.h: bool isKnownWindowsMSVCEnvironment() const { ../llvm/include/llvm/ADT/Triple.h: return isKnownWindowsMSVCEnvironment() || ../llvm/lib/Analysis/TargetLibraryInfo.cpp: if (T.isKnownWindowsMSVCEnvironment()) { ../llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp: if (getSubtargetInfo().getTargetTriple().isKnownWindowsMSVCEnvironment()) { ../llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp: if (!TT.isKnownWindowsMSVCEnvironment()) ../llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp: if (T.isKnownWindowsMSVCEnvironment() || T.isWindowsItaniumEnvironment()) { ../llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp: if (T.isKnownWindowsMSVCEnvironment() || T.isWindowsItaniumEnvironment()) { ../llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp: if (!T.isKnownWindowsMSVCEnvironment() && ../llvm/lib/IR/Mangler.cpp: if (TT.isKnownWindowsMSVCEnvironment()) ../llvm/lib/IR/Mangler.cpp: if (TT.isKnownWindowsMSVCEnvironment()) ../llvm/lib/IR/Mangler.cpp: if (!T.isKnownWindowsMSVCEnvironment()) ../llvm/lib/MC/MCWinCOFFStreamer.cpp: if (T.isKnownWindowsMSVCEnvironment()) { ../llvm/lib/MC/MCWinCOFFStreamer.cpp: if (!T.isKnownWindowsMSVCEnvironment() && ByteAlignment > 1) { ../llvm/lib/Target/X86/X86Subtarget.h: return TargetTriple.isKnownWindowsMSVCEnvironment();

Mainly, a user reported that the storage class of __real@XXX constant pool entries was incorrect when they used llc, but not clang, which overrides the module triple.

rnk commented 5 years ago

A few uses remain after r365387:

$ git grep isKnownWindowsMSVCEnvironment ../llvm ../clang ../clang/lib/Basic/TargetInfo.cpp: TheCXXABI.set(Triple.isKnownWindowsMSVCEnvironment() ../clang/lib/CodeGen/CGObjCGNU.cpp: CGF.CGM.getTarget().getTriple().isKnownWindowsMSVCEnvironment()) ../clang/lib/CodeGen/CodeGenModule.cpp: if (Context.getTargetInfo().getTriple().isKnownWindowsMSVCEnvironment() && ../clang/lib/Driver/ToolChains/CommonArgs.cpp: if (TC.getTriple().isKnownWindowsMSVCEnvironment()) { ../llvm/include/llvm/ADT/Triple.h: bool isKnownWindowsMSVCEnvironment() const { ../llvm/include/llvm/ADT/Triple.h: return isKnownWindowsMSVCEnvironment() || ../llvm/lib/Analysis/TargetLibraryInfo.cpp: if (T.isKnownWindowsMSVCEnvironment()) {

clang: The uses in clang are not that interesting because clang normalizes most of its sources for triples, and normalization turns unknown environments into MSVC environments. I think perhaps instead of normalizing everywhere, we might want to drop the "known" MSVC env portion of our checks.

Triple.h: Definition and declaration

TargetLibraryInfo.cpp: This guards extraction of the CRT library version, so I didn't remove it. There's no version to extract from an unknown environment.

pogo59 commented 5 years ago

Hm. Even if the .ll file has a different architecture, it overrides. It's hard to believe that's desirable, even if it warns. Maybe with an explicit -triple option, but not by default.

rnk commented 5 years ago

clang, which overrides the module triple

"overrides" as in normalizes the module triple, or "overrides" as in completely ignores it? If clang is imposing its own notion of the triple without considering what's in the IR file, that's... really bad.

It completely ignores it:

$ cat t.ll target triple = "x86_64-pc-win32" declare dllimport void @​my_function(double, double) define i32 @​main() { call void @​my_function(double 2.000000e+00, double 2.000000e+00) ret i32 0 }

$ clang -c t.ll warning: overriding the module target triple with x86_64-pc-windows-msvc19.21.27702 [-Woverride-module] 1 warning generated.

At least it warns, though. shrug

pogo59 commented 5 years ago

clang, which overrides the module triple

"overrides" as in normalizes the module triple, or "overrides" as in completely ignores it? If clang is imposing its own notion of the triple without considering what's in the IR file, that's... really bad.