[PATCH] D95458: [PowerPC] Exploit xxsplti32dx (constant materialization) for scalars

[Stefan Pintilie via Phabricator via cfe-commits]

stefanp added a comment.

Comments relate to just cleaning up the patch a little.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:8593
+  return !convertToNonDenormSingle(ArgAPFloat);
+}
+
----------------
I'm wondering if it would not be better to just inline this. It's just "not" of another call. That would simplify the patch a little.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:15874
+      return convertToNonDenormSingle(APFloatOfImm) ||
+             checkNonDenormCannotConvertToSingle(APFloatOfImm);
     }
----------------
Isn't this just :
```
return convertToNonDenormSingle(APFloatOfImm) ||
       !convertToNonDenormSingle(APFloatOfImm);
```
Which is always true?

Basically the logic is that we can now materialize without a load any `f32` or any `f64`.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.h:1321
   bool convertToNonDenormSingle(APFloat &ArgAPFloat);
+  bool checkNonDenormCannotConvertToSingle(APInt &ArgAPInt);
+  bool checkNonDenormCannotConvertToSingle(APFloat &ArgAPFloat);
----------------
Conanap wrote:
> stefanp wrote:
> > Is the APInt version of this function used anywere?
> > 
> Hm I don't think so, although I implemented it for consistency with `XXSPLTIDP` (`convertToNonDenormSingle`). I'll remove this if that is preferred.
nit:
Ok, unless other reviewers disagree, just remove it.


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

https://reviews.llvm.org/D95458