[PATCH] D35267: Pass Divergence Analysis data to selection DAG to drive divergence dependent instruction selection

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUISelLowering.cpp:781
+        if (RI->isPhysicalRegister(Reg)) {
+          return RI->getRegClass(AMDGPU::VGPR_32RegClassID)->contains(Reg) ||
+            RI->getRegClass(AMDGPU::VReg_64RegClassID)->contains(Reg) ||
----------------
alex-t wrote:
> rampitec wrote:
> > SIRegisterInfo::isVGPR()
> What should we do fro R600Subtarget?
Since we are not going to change selection in R600 that is practically non-important what we do for this case.


================
Comment at: lib/Target/AMDGPU/AMDGPUISelLowering.cpp:794
+            return true;
+          return DA->isDivergent(FLI->getValueFromVirtualReg(Reg));
+        }
----------------
alex-t wrote:
> rampitec wrote:
> > !DA || DA->isDivergent(...)
> > 
> > You are using getAnalysisIfAvailable, so it can be missing.
> if DA == nulptr in the case above we'd return ((bool)!DA) true?
> 
> maybe it's better return false for the targets that have no DA?
> 
> I mean " DA && DA->isDivegent()" if we have no DA we return false. In case we have, the returned value will be defined by the isDivergent result
That is conservatively correct to return true. Presumably targets w/o DA will have no use of the bit anyway, but if they are it is dangerous to assume uniformness.


https://reviews.llvm.org/D35267