[AMDGPU]: Fixes an invalid clamp selection pattern.

[tsymalla]

Summary

When running the tests on PowerPC and x86, the lit test GlobalISel/trunc.ll fails at the memory sanitize step. This seems to be due to wrong invalid logic (which matches even if it shouldn't) and likely missing variable initialisation."

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

74

I don't see why you need to uglify this. Can't you just reset MatchInfo before the second matcher?

[tsymalla]

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

74

The problem is: in the current implementation, if both of the matches fail, there is a fallthrough. If there is no min or max following the trunc (see the test case), the match will continue. I want to make sure at least one case exists. That's why I am double-checking here.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

92–107

Can you extract this into a separate function and call it in the passing cases instead?

[tsymalla]

[AMDGPU] Re-adding the changes from D93708 and fixing the clamp selection pattern.
This patch intends to re-add the changes from D93708 which were removed in d49efdc9696afee4b972c54bc3678b28c5700047 and should fix the issue adressed in this update.
This should fix the sanitizer builds and prevent applying the clamp in unwanted cases.

[tsymalla]

I extracted the corresponding parts into a separate lambda and improved the logic to only apply the combine in the corresponding cases where either a min-max or max-min pattern exists in the MIR.
As @Flakebi reverted the changes to prevent the sanitizer from failing, I re-added the changes to this revision.

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

protected:
   MachineRegisterInfo &MRI;
   CombinerHelper &Helper;
 
 public:
   AMDGPUPreLegalizerCombinerHelper(MachineIRBuilder &B, CombinerHelper &Helper)
       : B(B), MF(B.getMF()), MRI(*B.getMRI()), Helper(Helper){};
 
   struct ClampI64ToI16MatchInfo {
-    int64_t Cmp1;
-    int64_t Cmp2;
+    int64_t Cmp1 = 0;
+    int64_t Cmp2 = 0;
     Register Origin;
   };
 
   bool matchClampI64ToI16(MachineInstr &MI, MachineRegisterInfo &MRI,
                           MachineFunction &MF,
                           ClampI64ToI16MatchInfo &MatchInfo);
 
   void applyClampI64ToI16(MachineInstr &MI,
bool AMDGPUPreLegalizerCombinerHelper::matchClampI64ToI16(
 
   const LLT DstType = MRI.getType(MI.getOperand(0).getReg());
   if (DstType != LLT::scalar(16))
     return false;
 
   Register Base;
 
   // Try to match a combination of min / max MIR opcodes.
-  if (mi_match(MI.getOperand(1).getReg(), MRI, m_GSMin(m_Reg(Base), m_ICst(MatchInfo.Cmp1)))) {
-    if (!mi_match(Base, MRI, m_GSMax(m_Reg(MatchInfo.Origin), m_ICst(MatchInfo.Cmp2)))) {
-      return false;
-    }
+  bool continueMatch =
+      mi_match(MI.getOperand(1).getReg(), MRI,
+               m_GSMin(m_Reg(Base), m_ICst(MatchInfo.Cmp1))) &&
+      mi_match(Base, MRI,
+               m_GSMax(m_Reg(MatchInfo.Origin), m_ICst(MatchInfo.Cmp2)));
+
+  if (!continueMatch) {
+    continueMatch =
+        mi_match(MI.getOperand(1).getReg(), MRI,
+                 m_GSMax(m_Reg(Base), m_ICst(MatchInfo.Cmp1))) &&
+        mi_match(Base, MRI,
+                 m_GSMin(m_Reg(MatchInfo.Origin), m_ICst(MatchInfo.Cmp2)));
   }
 
-  if (mi_match(MI.getOperand(1).getReg(), MRI, m_GSMax(m_Reg(Base), m_ICst(MatchInfo.Cmp1)))) {
-    if (!mi_match(Base, MRI, m_GSMin(m_Reg(MatchInfo.Origin), m_ICst(MatchInfo.Cmp2)))) {
-      return false;
-    }
-  }
-   
+  if (!continueMatch) {
   const auto Cmp1 = MatchInfo.Cmp1;
   const auto Cmp2 = MatchInfo.Cmp2;
   const auto Diff = std::abs(Cmp2 - Cmp1);
 
   // If the difference between both comparison values is 0 or 1, there is no
   // need to clamp.
   if (Diff == 0 || Diff == 1)
     return false;
 
   const int64_t Min = std::numeric_limits<int16_t>::min();
   const int64_t Max = std::numeric_limits<int16_t>::max();
 
   // Check if the comparison values are between SHORT_MIN and SHORT_MAX.
   return ((Cmp2 >= Cmp1 && Cmp1 >= Min && Cmp2 <= Max) ||
           (Cmp1 >= Cmp2 && Cmp1 <= Max && Cmp2 >= Min));
 }
 
 // We want to find a combination of instructions that
 // gets generated when an i64 gets clamped to i16.
 // The corresponding pattern is:
 // G_MAX / G_MAX for i16 <= G_TRUNC i64.
 // This can be efficiently written as following:
 // v_cvt_pk_i16_i32 v0, v0, v1
 // v_med3_i32 v0, Clamp_Min, v0, Clamp_Max