DAG: ComputeNumSignBits from load range metadata

[arsenm]

Summary

The cases where the result type doesn't match the range type
are inadequately tested, but I'm not sure how to write such a
test. During the pre-legalize combine, any obviously optimizable
code gets handled so it's harder to test legalized extloads.

[arsenm]

ping

[nhaehnle]

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

3536–3537 ↗

(On Diff #149700)

First, this should probably use getSignedMin() and getSignedMax() instead of getLower() and getUpper(). An interesting test for this would be a range {i32 -1, i32 1}, which to my understanding only allows the values -1 and 0, so should have 32 sign bits, while the code as-is would say that it has 31 sign bits.

The other thing that worries me is what happens when there is a mismatch between the type used in the definition of the range metadata and the type returned by the SDNode. At the very least, there needs to be an assert to check against that case if we think it can't happen. If it can happen, it needs to be handled by taking the bit widths of the type of LD and of the APInts in the constant range into account.

test/CodeGen/AMDGPU/load-range-metadata-sign-bits.ll

6 ↗

(On Diff #149700)

Spelling: *metadata

Also, why can't we always use v_bfe_i32 for (ashr (shl X, Y), Y) (and v_bfe_u32 for (lshr (shl X, Y), Y)), independent of any metadata?

[nhaehnle]

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

3536–3537 ↗

(On Diff #149700)

I just double-checked, and at least the ConstantRange always has the same bit width for lower and upper bounds. But we still need to guard against that bit width being different from the bit width of the SDNode.

[arsenm]

Rebase, on top of baseline test commit. Also try to handle the unequal result size case, though poorly tested

[arsenm]

ping

[RKSimon]

LGTM - cheers

[arsenm]

47c68904a53a5aeeb2f7d506ac6be7d1a0fb951e

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

 #include "llvm/CodeGen/SelectionDAGNodes.h"
 #include "llvm/CodeGen/SelectionDAGTargetInfo.h"
 #include "llvm/CodeGen/TargetFrameLowering.h"
 #include "llvm/CodeGen/TargetLowering.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #include "llvm/CodeGen/ValueTypes.h"
 #include "llvm/IR/Constant.h"
+#include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/Metadata.h"
case ISD::INSERT_SUBVECTOR: {
     }
     if (!!DemandedSrcElts) {
       Tmp2 = ComputeNumSignBits(Src, DemandedSrcElts, Depth + 1);
       Tmp = std::min(Tmp, Tmp2);
     }
     assert(Tmp <= VTBits && "Failed to determine minimum sign bits");
     return Tmp;
   }
+  case ISD::LOAD: {
+    LoadSDNode *LD = cast<LoadSDNode>(Op);
+    if (const MDNode *Ranges = LD->getRanges()) {
+      if (DemandedElts != 1)
+        break;
+
+      ConstantRange CR = getConstantRangeFromMetadata(*Ranges);
+      if (VTBits > CR.getBitWidth()) {
+        switch (LD->getExtensionType()) {
+        case ISD::SEXTLOAD:
+          CR = CR.signExtend(VTBits);
+          break;
+        case ISD::ZEXTLOAD:
+          CR = CR.zeroExtend(VTBits);
+          break;
+        default:
+          break;
+        }
+      }
+
+      if (VTBits != CR.getBitWidth())
+        break;
+      return std::min(CR.getSignedMin().getNumSignBits(),
+                      CR.getSignedMax().getNumSignBits());
+    }
+
+    break;
+  }
   case ISD::ATOMIC_CMP_SWAP:
   case ISD::ATOMIC_CMP_SWAP_WITH_SUCCESS:
   case ISD::ATOMIC_SWAP:
   case ISD::ATOMIC_LOAD_ADD:
   case ISD::ATOMIC_LOAD_SUB:
   case ISD::ATOMIC_LOAD_AND:
   case ISD::ATOMIC_LOAD_CLR:
   case ISD::ATOMIC_LOAD_OR:

llvm/test/CodeGen/AMDGPU/load-range-metadata-sign-bits.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefix=GCN %s
 
 define i32 @range_metadata_sext_i8_signed_range_i32(ptr addrspace(1) %ptr) {
 ; GCN-LABEL: range_metadata_sext_i8_signed_range_i32:
 ; GCN:       ; %bb.0:
 ; GCN-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GCN-NEXT:    global_load_dword v0, v[0:1], off glc
 ; GCN-NEXT:    s_waitcnt vmcnt(0)
-; GCN-NEXT:    v_bfe_i32 v0, v0, 0, 8
 ; GCN-NEXT:    s_setpc_b64 s[30:31]
   %val = load volatile i32, ptr addrspace(1) %ptr, align 4, !range !0 ; [-127, 128)
   %shl = shl i32 %val, 24
   %ashr = ashr i32 %shl, 24
   ret i32 %ashr
 }
 
 define i32 @range_metadata_sext_upper_range_limited_i32(ptr addrspace(1) %ptr) {
; GCN-NEXT:    s_setpc_b64 s[30:31]
   %ashr = ashr i32 %shl, 25
   ret i32 %ashr
 }
 
 define i32 @range_metadata_sextload_i8_signed_range_i4_i32(ptr addrspace(1) %ptr) {
 ; GCN-LABEL: range_metadata_sextload_i8_signed_range_i4_i32:
 ; GCN:       ; %bb.0:
 ; GCN-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
-; GCN-NEXT:    global_load_ubyte v0, v[0:1], off glc
+; GCN-NEXT:    global_load_sbyte v0, v[0:1], off glc
 ; GCN-NEXT:    s_waitcnt vmcnt(0)
-; GCN-NEXT:    v_lshlrev_b16_e32 v0, 11, v0
-; GCN-NEXT:    v_ashrrev_i16_e32 v0, 11, v0
-; GCN-NEXT:    v_bfe_i32 v0, v0, 0, 16
 ; GCN-NEXT:    s_setpc_b64 s[30:31]
   %load = load volatile i8, ptr addrspace(1) %ptr, align 1, !range !4
   %shl = shl i8 %load, 3
   %ashr = ashr i8 %shl, 3
   %ext = sext i8 %ashr to i32
   ret i32 %ext
 }
 

 }
 
 define i32 @range_metadata_i32_neg1_to_1(ptr addrspace(1) %ptr) {
 ; GCN-LABEL: range_metadata_i32_neg1_to_1:
 ; GCN:       ; %bb.0:
 ; GCN-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GCN-NEXT:    global_load_dword v0, v[0:1], off glc
 ; GCN-NEXT:    s_waitcnt vmcnt(0)
-; GCN-NEXT:    v_bfe_i32 v0, v0, 0, 1
 ; GCN-NEXT:    s_setpc_b64 s[30:31]
   %val = load volatile i32, ptr addrspace(1) %ptr, align 4, !range !6
   %shl = shl i32 %val, 31
   %ashr = ashr i32 %shl, 31
   ret i32 %ashr
 }
 
 define i64 @range_metadata_sext_i8_signed_range_i64(ptr addrspace(1) %ptr) {

 }
 
 define i64 @range_metadata_sext_i32_signed_range_i64(ptr addrspace(1) %ptr) {
 ; GCN-LABEL: range_metadata_sext_i32_signed_range_i64:
 ; GCN:       ; %bb.0:
 ; GCN-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GCN-NEXT:    global_load_dwordx2 v[0:1], v[0:1], off glc
 ; GCN-NEXT:    s_waitcnt vmcnt(0)
-; GCN-NEXT:    v_lshlrev_b64 v[0:1], 31, v[0:1]
-; GCN-NEXT:    v_ashrrev_i64 v[0:1], 31, v[0:1]
 ; GCN-NEXT:    s_setpc_b64 s[30:31]
   %val = load volatile i64, ptr addrspace(1) %ptr, align 4, !range !7
   %shl = shl i64 %val, 31
   %ashr = ashr i64 %shl, 31
   ret i64 %ashr
 }
 
 define i64 @range_metadata_sext_i33_signed_range_i64(ptr addrspace(1) %ptr) {
 ; GCN-LABEL: range_metadata_sext_i33_signed_range_i64:
 ; GCN:       ; %bb.0:
 ; GCN-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GCN-NEXT:    global_load_dwordx2 v[0:1], v[0:1], off glc
 ; GCN-NEXT:    s_waitcnt vmcnt(0)
-; GCN-NEXT:    v_lshlrev_b64 v[0:1], 30, v[0:1]
-; GCN-NEXT:    v_ashrrev_i64 v[0:1], 30, v[0:1]
 ; GCN-NEXT:    s_setpc_b64 s[30:31]
   %val = load volatile i64, ptr addrspace(1) %ptr, align 4, !range !8
   %shl = shl i64 %val, 30
   %ashr = ashr i64 %shl, 30
   ret i64 %ashr
 }
 
 !0 = !{i32 -127, i32 128}
 !1 = !{i32 -127, i32 256}
 !2 = !{i32 -255, i32 128}
 !3 = !{i32 -127, i32 -64}
 !4 = !{i8 -15, i8 16}
 !5 = !{i25 -127, i25 128}
 !6 = !{i32 -1, i32 1}
 !7 = !{i64 -4294967295, i64 4294967296}
 !8 = !{i64 -8589934591, i64 8589934592}