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

[Quuxplusone]

Bugzilla Link	PR42491
Status	NEW
Importance	P enhancement
Reported by	Reid Kleckner (rnk@google.com)
Reported on	2019-07-02 15:09:12 -0700
Last modified on	2019-07-09 12:58:47 -0700
Version	trunk
Hardware	PC Windows NT
CC	dmajor@bugmail.cc, llvm-bugs@lists.llvm.org, paul_robinson@playstation.sony.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

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.

[Quuxplusone]

> 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.

[Quuxplusone]

(In reply to Paul Robinson from comment #1)
> > 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*

[Quuxplusone]

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.

[Quuxplusone]

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.