[PATCH] D93239: GlobalISel: Return APInt from getConstantVRegVal

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1413
     return false; // Leave it to the legalizer to lower it to a libcall.
-  unsigned KnownLen = LenVRegAndVal->Value;
+  unsigned KnownLen = LenVRegAndVal->Value.getZExtValue();
 
----------------
arsenm wrote:
> paquette wrote:
> > Why `getZExtValue` here rather than `getSExtValue`? Looks more correct considering it's an unsigned, but the old behaviour would have gotten the sign extended value.
> Yes, I'm assuming this is supposed to be an unsigned value
Yeah, as a length arg this should always be unsigned.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1656
 
-  const uint64_t C1Val = MaybeImmVal->Value;
+  const uint64_t C1Val = MaybeImmVal->Value.getSExtValue();
 
----------------
This should be ZExt though?


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:4665
     return false;
-  unsigned LaneIdx = VRegAndVal->Value;
+  unsigned LaneIdx = VRegAndVal->Value.getSExtValue();
 
----------------
ZExt


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp:441
     return None;
-  uint64_t C = ValAndVReg->Value;
+  uint64_t C = ValAndVReg->Value.getSExtValue();
   if (isLegalArithImmed(C))
----------------
ZExt


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

https://reviews.llvm.org/D93239