[PATCH] D110076: [AMDGPU][GlobalISel] Code quality: Combine V_RSQ

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstructions.td:830
 
+let OtherPredicates = [UnsafeFPMath] in {
+
----------------
matejam wrote:
> arsenm wrote:
> > I don't understand this change. Are you saying this is a dead selection pattern for the DAG? Should we be doing this in the combiner instead and just delete this? That way we could consider the fast math flags and not rely on the function attribute
> I am, with or without this pattern SDAG combines v_sqrt + v_rcp into v_rsq. I'm not sure which would be better to leave this as a pattern or write a combiner for this. In fact SDAG doesn't even need any flags to combine into v_rsq. 
If this is a dead pattern in the DAG, I would just delete it. When you say without flags, I assume you mean with the unsafe attribute? I'm a bit worried this pattern is just broken as-is. This depends on the denormal mode, and also could be augmented to use the per-instruction flags. I think it's safer to move this to a combine.


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

https://reviews.llvm.org/D110076