[PATCH] D133593: [AMDGPU] Always select s_cselect_b32 for uniform 'select' SDNode

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:4002
 
+  virtual bool checkForPhysRegDependency(SDNode *Def, SDNode *User, unsigned Op,
+                                      const TargetRegisterInfo *TRI,
----------------
Need a description of the function.


================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:4003
+  virtual bool checkForPhysRegDependency(SDNode *Def, SDNode *User, unsigned Op,
+                                      const TargetRegisterInfo *TRI,
+                                      const TargetInstrInfo *TII,
----------------
Alignment is off.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:119
   unsigned Reg = cast<RegisterSDNode>(User->getOperand(1))->getReg();
+  if (TLI.checkForPhysRegDependency(Def, User, Op, TRI, TII, PhysReg, Cost))
+    return;
----------------
Place it after the check for isVirtualRegister below?


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12983
+bool SITargetLowering::checkForPhysRegDependency(SDNode *Def, SDNode *User, unsigned Op,
+                                      const TargetRegisterInfo *TRI,
+                                      const TargetInstrInfo *TII,
----------------
Alignment.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12998
+  const MCInstrDesc &II = TII->get(MDef->getMachineOpcode());
+  if (II.isCompare() && II.hasImplicitDefOfPhysReg(AMDGPU::SCC)) {
+    PhysReg = AMDGPU::SCC;
----------------
We may actually later extend it beyond compares and to include VCC.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.h:483
+  bool checkForPhysRegDependency(SDNode *Def, SDNode *User, unsigned Op,
+                                      const TargetRegisterInfo *TRI,
+                                      const TargetInstrInfo *TII,
----------------
Alignment.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:935
+  if (RC == &AMDGPU::SCC_CLASSRegClass)
+    return ST.isWave32() ? &AMDGPU::SReg_32_XEXEC_HIRegClass
+                         : &AMDGPU::SReg_64_XEXECRegClass;
----------------
getWaveMaskRegClass()? The difference that it returns SReg_32_XM0_XEXECRegClass and not SReg_32_XEXEC_HIRegClass.


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

https://reviews.llvm.org/D133593