[llvm] [llc] set canonical triple in module (PR #203725)

[Matt Arsenault via llvm-commits]

================
@@ -0,0 +1,16 @@
+;; Verify that llc correctly sets the module triple when one is present.
+
+; RUN: llc -march=aarch64                   -stop-after=finalize-isel -o - %s | FileCheck %s --check-prefix=MARCH
+; RUN: llc -mtriple=aarch64-unknown-unknown -stop-after=finalize-isel -o - %s | FileCheck %s --check-prefix=MTRIPLE
+; RUN: llc                                  -stop-after=finalize-isel -o - %s | FileCheck %s --check-prefix=MODULE
+
+; REQUIRES: aarch64-registered-target
+; REQUIRES: x86-registered-target
+
+; MARCH: target triple = "aarch64-unknown-linux-gnu"
+; MTRIPLE: target triple = "aarch64-unknown-unknown"
+; MODULE: target triple = "x86_64-unknown-linux-gnu"
+
+target triple = "x86_64-unknown-linux-gnu"
----------------
arsenm wrote:

> I vaguely remember when we intentionally chose to have llc override the module triple. At the time, the use case was to be able to dump IR, compile it with llc, then compile it again with a new triple that overrides the module triple, perhaps with different ISA features, to observe the result.

I have never wanted to do this. IR is target specific from construction, and running another target's IR through llc is just looking for less-than-useful bug reports. This is only useful as a stochastic fuzzing technique for unexpected inputs. The realistic use case for triple-from-command-line is when you want to compile multiple targets in the same lit test, like x86 tests 32/64 bit case. In which case the file shouldn't contain a triple and llc should set it.

> I think, to your point, llc is a developer tool, so why shouldn't llc -mtriple=x override the module triple? SInce we have that support, why shouldn't it rewrite the module triple? Why is flexibility bad for developer experience?

Because of the element of surprise. I do not expect an arbitrary IR sample to work across all targets, especially if that's IR from any real program. 

> However, I don't think it's reasonable to ask @AlexMaclean to make llc emit an error on triple mismatch, and then to clean up our entire lit test suite, add release notes, document user-visible breaking changes, etc, just to set the module triple for generic tests.

Doing that would be an improvement over the status quo, I don't think this is. The module's triple *should* be set, but not if it conflicts with the command line's. I also don't think this would be a particularly pervasive change, did you try?

> This blocks https://github.com/llvm/llvm-project/pull/204637 since the CodeGen/Generic tests do not set the triple in the module and without this change the default triple is not set in the module either.

I didn't follow the specific here, but CodeGen/Generic tests simply shouldn't exist. You cannot have a stable test that works on an arbitrary backends, and this is a frequent source of build bot breakage. These should be sharded out into individual targets (though the common case here just needs to pick one as a sample). 

https://github.com/llvm/llvm-project/pull/203725