[PATCH] D155626: [DAG][AArch64] Fix truncated vscale constant types

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:5754
     if (OpOpcode == ISD::VSCALE && !NewNodesMustHaveLegalTypes)
-      return getVScale(DL, VT, N1.getConstantOperandAPInt(0));
+      return getVScale(
+          DL, VT,
----------------
sdesmalen wrote:
> `SelectionDAG::getVScale` does the right thing by truncating the APInt value, but it only does so after the assert:
> 
>   assert(MulImm.getSignificantBits() <= VT.getSizeInBits() &&
>             "Immediate does not fit VT");
> 
> which fails because for `MulImm=1`, `MulImm.getSignificantBits() ` results in `2`. This happens because it adds the sign bit, which for a 1-bit value is nonsensical. I think the better thing to do is to fix the assert itself.
I'm not sure it wouldn't be better for getVScale to only accept APInt of the correct bitwidth, to be more precise. I couldn't see any other code that looked like the APInt and the VT could be a mismatch.

The new version just adds an override for i1 to the assert. Let me know what you think.


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

https://reviews.llvm.org/D155626