[PATCH] D123284: [ArgPromotion][Attributor] Update min-legal-vector-width when do promotion

Tue Apr 19 08:26:29 PDT 2022

pengfei added a comment.

In D123284#3458997 <https://reviews.llvm.org/D123284#3458997>, @jdoerfert wrote:

>>   Finally, I'd suggest to avoid the undef and UB.
>>
>> Both argument promotion and lining are legal behavior. I think there's no undef/UB for single pass here.
>
> To make it clearer and avoid similar situations in the future:
>
> 1. Argument promotion was legal in that test case.
> 2. Inlining would have been legal, it wasn't happening for the test case.
> 3. Passes with UB are bugs, I was not referring to that.
> 4. The functions in the test could not have been executed without triggering UB. Hence, it would have been valid to remove the call and place an unreachable there instead. Valid, but obviously not what you wanted to test.

No sure if I understood it correctly. Do you mean "the test case" by https://godbolt.org/z/zo3hba8xW? The case was drastically reduced from a big IR. It won't generate valid assembly but demonstrates the problem. The ABI mismatch can be proved by another test https://godbolt.org/z/KnsPorvKa

> Talking about tests, I'm confused why the ArgumentPromotion/X86/min-legal-vector-width test is not affected. The Attributor ones are "simply copies".

Please note the different between `IS__TUNIT_OPM: attributes` and `IS__TUNIT_NPM: attributes`. The former has one more attribute than the latter, because "min-legal-vector-width"="256" in `ATTR4` turns into "min-legal-vector-width"="512", which is the same with `ATTR3`. So we reduced one, then `ATTR6` -> `ATTR5` and `ATTR7` -> `ATTR6`
But it seems we lack a test for ArgPromotion, so I think I should add the previous test back.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123284