[PATCH] D155638: [llvm-reduce] Reduce function calling convention

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D155638#4516642 <https://reviews.llvm.org/D155638#4516642>, @arsenm wrote:

> In D155638#4515831 <https://reviews.llvm.org/D155638#4515831>, @nikic wrote:
>
>> It's an IR centric view. Calling convention is generally irrelevant for middle-end tests and should be omitted from them.
>
> This isn't a middle end test, it's a general IR tool

Sure, but it works on IR, so it should do an IR focused reduction.

I think we have a lot of precedent for this. For example, we reduce away the "dso_local" flag, which is a simplification on the IR level, but likely makes codegen more complex. Similarly, we reduce globals to external linkage -- again, this likely makes codegen more complex. Etc.

llvm-reduce at least right now favors producing minimal IR, not producing IR that gives you the minimum number of asm instructions after codegen.

If we wanted llvm-reduce to optimize for codegen tests, it should probably do things like add `nounwind` attributes to functions to avoid CFI, convert globals to internal linkage to avoid relocations, etc.

> Requiring additional complexity in interestingness tests is not good. The ideal interestingness test is just run the crashing program, I shouldn't have to hack around odd tooling decisions.

You don't need to explicitly add something to the interestingness test. If the crash reproduces only with a specific CC, then you'll get that CC. If it doesn't require a specific CC, then it's okay to remove it.

> The calling convention can and does place IR constraints which this is not considering, and we should try to avoid failing the verifier in any reduction.

I do agree with this part -- do you have any example where removing the calling convention would cause a verifier error? I would have assumed that the default cc doesn't have any special restrictions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155638