[PATCH] D88773: Reland "[WebAssembly] Emulate v128.const efficiently""

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added a comment.

I can confirm that at least `llvm/test/CodeGen/WebAssembly/simd-build-vector.ll` is good on AIX (big-endian Power) with this patch (at least with the `maskTrailingOnes<uint64_t>(LaneBits)`) change.



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1600
+        I64s[I / HalfLanes] |= Val << Shift;
+        ConstLaneMasks[I / HalfLanes] |= ((1ULL << LaneBits) - 1) << Shift;
+      }
----------------
tlively wrote:
> hubert.reinterpretcast wrote:
> > Can `LaneBits` be 64?
> Yes, if the vector is already an v2i64.
Okay, I suggest using `maskTrailingOnes<uint64_t>(LaneBits)` to avoid undefined behaviour:
```
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index ec62f2a..b2913b6 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -30,8 +30,8 @@
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsWebAssembly.h"
 #include "llvm/Support/Debug.h"
-#include "llvm/Support/Endian.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetOptions.h"
 using namespace llvm;
@@ -1597,7 +1597,8 @@ SDValue WebAssemblyTargetLowering::LowerBUILD_VECTOR(SDValue Op,
         auto Shift = LaneBits * (I % HalfLanes);
         auto Val = cast<ConstantSDNode>(Lane.getNode())->getZExtValue();
         I64s[I / HalfLanes] |= Val << Shift;
-        ConstLaneMasks[I / HalfLanes] |= ((1ULL << LaneBits) - 1) << Shift;
+        ConstLaneMasks[I / HalfLanes] |= maskTrailingOnes<uint64_t>(LaneBits)
+                                         << Shift;
       }
     }
     // Check whether all constant lanes in the second half of the vector are
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88773