[PATCH] D104836: [PowerPC] Combine 64-bit bswap(load) without LDBRX

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 24 11:20:11 PDT 2021


spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

See inline for a couple of small adjustments, otherwise LGTM. 
I'm not up-to-speed on current PPC targets though, so might want to wait for a 2nd opinion.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:15254
+    // Can't split volatile loads.
+    if (LD->isVolatile())
+      return SDValue();
----------------
I forgot to mention atomics (add one more test...).
I think we want to use "LD->isSimple()" to be safe.


================
Comment at: llvm/test/CodeGen/PowerPC/ld-bswap64-no-ldbrx.ll:19
+
+define i64 @misaligned_volatile(i64* %p) {
+; CHECK-LABEL: misaligned_volatile:
----------------
IIUC, we are ok doing the transform with any alignment, so this test isn't capturing that. 

Please add a misaligned-only test or adjust the first test to have "align 1" so it can verify that we perform the fold independent of alignment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104836



More information about the llvm-commits mailing list