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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

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

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



================
Comment at: llvm/lib/IR/Attributes.cpp:2041
+  }
+}
----------------
Can't this function walk the arguments of `Fn` and determine `Width` if necessary?


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2499
+        LargestVectorWidth = std::max(
+            LargestVectorWidth, VT->getPrimitiveSizeInBits().getKnownMinSize());
+
----------------
Shouldn't we use the DataLayout for size questions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123284