[PATCH] D110527: [llvm-reduce] Add MIR support.

[Markus Lavin via Phabricator via llvm-commits]

markus added a comment.

In D110527#3121954 <https://reviews.llvm.org/D110527#3121954>, @bjope wrote:

> In D110527#3121935 <https://reviews.llvm.org/D110527#3121935>, @reames wrote:
>
>> In D110527#3121915 <https://reviews.llvm.org/D110527#3121915>, @bjope wrote:
>>
>>> The IR part can often be eliminated from a MIR based test case. But sure, one could argue that when running llvm-reduce it could be mandatory to add an IR section in the .mir file first (but then I think one needs to also add the function prototype).
>>
>> I can maybe see this as an argument for allowing override of the triple, but not as an argument for not defaulting to the triple if present.  Frankly, I find even the override case unconvincing.
>
> Yes. If there is a target triple in the IR then overriding it might be weird. I don't remember exactly how it works in llc, but as a user I guess I'd probably would assume that the behavior would be the same between the tools when using cl option vs having the triple in the IR, or having both.

The way I see it is more or less what @bjope already said, often with MIR tests the IR section is eliminated (e.g. llvm/test/tools/llvm-reduce/mir/instr-reduce.mir) and you want to be able to work with that so some kind of option for setting the triple is needed.
If the IR section specifies a triple and/or data layout then it makes sense that that is used and `-mtriple` is not required. If both are specified then either `-mtriple` should have precedence or an error message should be printed. Perhaps the latter option is preferred to avoid confusion.
Besides that there was no deep thinking behind it other than trying to have MIR support working with the least amount of code changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110527/new/

https://reviews.llvm.org/D110527