[llvm] 734881a - [Hexagon] Fix range checks for immediate operands

[Krzysztof Parzyszek via llvm-commits]

Author: Krzysztof Parzyszek
Date: 2023-06-19T08:22:41-07:00
New Revision: 734881a6d546c6065a4aa43c3d876337b8b2a263

URL: https://github.com/llvm/llvm-project/commit/734881a6d546c6065a4aa43c3d876337b8b2a263
DIFF: https://github.com/llvm/llvm-project/commit/734881a6d546c6065a4aa43c3d876337b8b2a263.diff

LOG: [Hexagon] Fix range checks for immediate operands

The output assembly (textual) contains the instruction
  r29 = add(r29,#4294967136)
The value 4294967136 is -160 when interpreted as a signed 32-bit
integer, so it fits in the range of the immediate operand without
a constant extender. The range check in HexagonInstrInfo was putting
the operand value into an int variable, reporting no need for an
extender. This resulted in a packet with 4 instructions, including
the "add". The corresponding check in HexagonMCInstrInfo was using
an int64_t variable, causing the range check to fail, and an extender
to be emitted when lowering to MCInst, resulting in a packet with
too many instructions.

Added: 
    llvm/test/CodeGen/Hexagon/imm-range-check.ll

Modified: 
    llvm/lib/Target/Hexagon/HexagonInstrInfo.cpp
    llvm/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/Hexagon/HexagonInstrInfo.cpp b/llvm/lib/Target/Hexagon/HexagonInstrInfo.cpp
index 55ddad2378ac9..15a09195eaca2 100644
--- a/llvm/lib/Target/Hexagon/HexagonInstrInfo.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonInstrInfo.cpp
@@ -2177,11 +2177,17 @@ bool HexagonInstrInfo::isConstExtended(const MachineInstr &MI) const {
   // have 'isExtended' flag set.
   assert(MO.isImm() && "Extendable operand must be Immediate type");
 
-  int MinValue = getMinValue(MI);
-  int MaxValue = getMaxValue(MI);
-  int ImmValue = MO.getImm();
-
-  return (ImmValue < MinValue || ImmValue > MaxValue);
+  int64_t Value = MO.getImm();
+  if ((F >> HexagonII::ExtentSignedPos) & HexagonII::ExtentSignedMask) {
+    int32_t SValue = Value;
+    int32_t MinValue = getMinValue(MI);
+    int32_t MaxValue = getMaxValue(MI);
+    return SValue < MinValue || SValue > MaxValue;
+  }
+  uint32_t UValue = Value;
+  uint32_t MinValue = getMinValue(MI);
+  uint32_t MaxValue = getMaxValue(MI);
+  return UValue < MinValue || UValue > MaxValue;
 }
 
 bool HexagonInstrInfo::isDeallocRet(const MachineInstr &MI) const {

diff  --git a/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.cpp b/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.cpp
index 0fd002ac56909..9cf004cf4c9a5 100644
--- a/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.cpp
+++ b/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.cpp
@@ -568,12 +568,20 @@ bool HexagonMCInstrInfo::isConstExtended(MCInstrInfo const &MCII,
   if (isa<HexagonMCExpr>(MO.getExpr()) &&
       HexagonMCInstrInfo::mustNotExtend(*MO.getExpr()))
     return false;
+
   int64_t Value;
   if (!MO.getExpr()->evaluateAsAbsolute(Value))
     return true;
-  int MinValue = HexagonMCInstrInfo::getMinValue(MCII, MCI);
-  int MaxValue = HexagonMCInstrInfo::getMaxValue(MCII, MCI);
-  return (MinValue > Value || Value > MaxValue);
+  if (HexagonMCInstrInfo::isExtentSigned(MCII, MCI)) {
+    int32_t SValue = Value;
+    int32_t MinValue = HexagonMCInstrInfo::getMinValue(MCII, MCI);
+    int32_t MaxValue = HexagonMCInstrInfo::getMaxValue(MCII, MCI);
+    return SValue < MinValue || SValue > MaxValue;
+  }
+  uint32_t UValue = Value;
+  uint32_t MinValue = HexagonMCInstrInfo::getMinValue(MCII, MCI);
+  uint32_t MaxValue = HexagonMCInstrInfo::getMaxValue(MCII, MCI);
+  return UValue < MinValue || UValue > MaxValue;
 }
 
 bool HexagonMCInstrInfo::isCanon(MCInstrInfo const &MCII, MCInst const &MCI) {

diff  --git a/llvm/test/CodeGen/Hexagon/imm-range-check.ll b/llvm/test/CodeGen/Hexagon/imm-range-check.ll
new file mode 100644
index 0000000000000..290a5b69d3334
--- /dev/null
+++ b/llvm/test/CodeGen/Hexagon/imm-range-check.ll
@@ -0,0 +1,26 @@
+; RUN: llc -march=hexagon -filetype=obj < %s | llvm-objdump -d - | FileCheck %s
+
+; The output assembly (textual) contains the instruction
+;   r29 = add(r29,#4294967136)
+; The value 4294967136 is -160 when interpreted as a signed 32-bit
+; integer, so it fits in the range of the immediate operand without
+; a constant extender. The range check in HexagonInstrInfo was putting
+; the operand value into an int variable, reporting no need for an
+; extender. This resulted in a packet with 4 instructions, including
+; the "add". The corresponding check in HexagonMCInstrInfo was using
+; an int64_t variable, causing an extender to be emitted when lowering
+; to MCInst, and resulting in a packet with 5 instructions.
+
+; Check that this doesn't crash.
+; CHECK: r29 = add(r29,#-0xa0)
+
+target triple = "hexagon-unknown-linux-gnu"
+
+define float @f0() {
+b0:
+  %v0 = alloca i8, i32 0, align 1
+  %v1 = alloca float, i32 -42, align 4
+  %v2 = load float, ptr %v1, align 4
+  store i8 0, ptr %v0, align 1
+  ret float %v2
+}