Expand masked mem intrinsics correctly wrt big-endian

[markus]

Summary

Need to take endianness into account when doing vector to scalar casts such as

%bc = bitcast <8 x i1> %v to i8

Companion review for https://reviews.llvm.org/D94867
Upload in response to https://lists.llvm.org/pipermail/llvm-dev/2021-January/147862.html
Attempting to document the actual memory layout rules for vectors in https://reviews.llvm.org/D94964

[dmgreen]

Thanks for the patch. There is a patch to make MVE consistent with the rest of MVE in D94867. This will need rebasing on top of that, with update tests to make the two consistent again.

llvm/lib/Transforms/Scalar/ScalarizeMaskedMemIntrin.cpp

321–322

It's a shame we can't just use extract elements - that would simplify this a lot!

[markus]

In D94765#2503378, @dmgreen wrote:

Thanks for the patch. There is a patch to make MVE consistent with the rest of MVE in D94867. This will need rebasing on top of that, with update tests to make the two consistent again.

Thank you for looking into this.

llvm/lib/Transforms/Scalar/ScalarizeMaskedMemIntrin.cpp

321–322

It's a shame we can't just use extract elements - that would simplify this a lot!

I fully agree.

[lebedev.ri]

Tests missing.

[bjope]

Add tests using big endian datalayout.

Also updated test checks in the thumb codegen test cases that is impacted by
this change. Not sure if this patch should be parent or child to D94867 (as
both patches need to land at the same time to not break big-endian ARM.

[dmgreen]

Yeah thanks. I've added a link

[dmgreen]

OK. Thanks for the commit. This LGTM.

Are you happy for me to commit this and D94867 together, to keep them in-sync?

[bjope]

In D94765#2548276, @dmgreen wrote:

OK. Thanks for the commit. This LGTM.

Are you happy for me to commit this and D94867 together, to keep them in-sync?

(answering on behalf of Markus who will be out-of-office for a couple of weeks)

That sounds great! Thanks for all the help with this problem.

llvm/lib/Transforms/Scalar/ScalarizeMaskedMemIntrin.cpp

for (unsigned i = 0; i != NumElts; ++i) {
     Constant *CElt = C->getAggregateElement(i);
     if (!CElt || !isa<ConstantInt>(CElt))
       return false;
   }
 
   return true;
 }
 
+static unsigned adjustForEndian(const DataLayout &DL, unsigned VectorWidth,
+                                unsigned Idx) {
+  return DL.isBigEndian() ? VectorWidth - 1 - Idx : Idx;
+}
+
 // Translate a masked load intrinsic like
 // <16 x i32 > @llvm.masked.load( <16 x i32>* %addr, i32 align,
 //                               <16 x i1> %mask, <16 x i32> %passthru)
 // to a chain of basic blocks, with loading element one-by-one if
 // the appropriate mask bit is set
 //
 //  %1 = bitcast i8* %addr to i32*
 //  %2 = extractelement <16 x i1> %mask, i32 0

 //  %9 = insertelement <16 x i32> %res.phi.else, i32 %8, i32 1
 //  br label %else2
 //
 // else2:                                          ; preds = %else, %cond.load1
 //  %res.phi.else3 = phi <16 x i32> [ %9, %cond.load1 ], [ %res.phi.else, %else ]
 //  %10 = extractelement <16 x i1> %mask, i32 2
 //  br i1 %10, label %cond.load4, label %else5
 //
-static void scalarizeMaskedLoad(CallInst *CI, bool &ModifiedDT) {
+static void scalarizeMaskedLoad(const DataLayout &DL, CallInst *CI,
+                                bool &ModifiedDT) {
   Value *Ptr = CI->getArgOperand(0);
   Value *Alignment = CI->getArgOperand(1);
   Value *Mask = CI->getArgOperand(2);
   Value *Src0 = CI->getArgOperand(3);
 
   const Align AlignVal = cast<ConstantInt>(Alignment)->getAlignValue();
   VectorType *VecType = cast<FixedVectorType>(CI->getType());
 
for (unsigned Idx = 0; Idx < VectorWidth; ++Idx) {
     //
     //  %res.phi.else3 = phi <16 x i32> [ %11, %cond.load1 ], [ %res.phi.else, %else ]
     //  %mask_1 = and i16 %scalar_mask, i32 1 << Idx
     //  %cond = icmp ne i16 %mask_1, 0
     //  br i1 %mask_1, label %cond.load, label %else
     //
     Value *Predicate;
     if (VectorWidth != 1) {
-      Value *Mask = Builder.getInt(APInt::getOneBitSet(VectorWidth, Idx));
+      Value *Mask = Builder.getInt(APInt::getOneBitSet(
+          VectorWidth, adjustForEndian(DL, VectorWidth, Idx)));
       Predicate = Builder.CreateICmpNE(Builder.CreateAnd(SclrMask, Mask),
                                        Builder.getIntN(VectorWidth, 0));
     } else {
       Predicate = Builder.CreateExtractElement(Mask, Idx);
     }
 
     // Create "cond" block
     //

 //   br i1 %5, label %cond.store1, label %else2
 //
 // cond.store1:                                      ; preds = %else
 //   %6 = extractelement <16 x i32> %val, i32 1
 //   %7 = getelementptr i32* %1, i32 1
 //   store i32 %6, i32* %7
 //   br label %else2
 //   . . .
-static void scalarizeMaskedStore(CallInst *CI, bool &ModifiedDT) {
+static void scalarizeMaskedStore(const DataLayout &DL, CallInst *CI,
+                                 bool &ModifiedDT) {
   Value *Src = CI->getArgOperand(0);
   Value *Ptr = CI->getArgOperand(1);
   Value *Alignment = CI->getArgOperand(2);
   Value *Mask = CI->getArgOperand(3);
 
   const Align AlignVal = cast<ConstantInt>(Alignment)->getAlignValue();
   auto *VecType = cast<VectorType>(Src->getType());
 
for (unsigned Idx = 0; Idx < VectorWidth; ++Idx) {
       Value *OneElt = Builder.CreateExtractElement(Src, Idx);
       Value *Gep = Builder.CreateConstInBoundsGEP1_32(EltTy, FirstEltPtr, Idx);
       Builder.CreateAlignedStore(OneElt, Gep, AdjustedAlignVal);
     }
     CI->eraseFromParent();
     return;
   }
 
   // If the mask is not v1i1, use scalar bit test operations. This generates
   // better results on X86 at least.
   Value *SclrMask;
   if (VectorWidth != 1) {
     Type *SclrMaskTy = Builder.getIntNTy(VectorWidth);
     SclrMask = Builder.CreateBitCast(Mask, SclrMaskTy, "scalar_mask");
   }
 
   for (unsigned Idx = 0; Idx < VectorWidth; ++Idx) {
     // Fill the "else" block, created in the previous iteration
     //
     //  %mask_1 = and i16 %scalar_mask, i32 1 << Idx
     //  %cond = icmp ne i16 %mask_1, 0
     //  br i1 %mask_1, label %cond.store, label %else
     //
     Value *Predicate;
     if (VectorWidth != 1) {
-      Value *Mask = Builder.getInt(APInt::getOneBitSet(VectorWidth, Idx));
+      Value *Mask = Builder.getInt(APInt::getOneBitSet(
+          VectorWidth, adjustForEndian(DL, VectorWidth, Idx)));
       Predicate = Builder.CreateICmpNE(Builder.CreateAnd(SclrMask, Mask),
                                        Builder.getIntN(VectorWidth, 0));
     } else {
       Predicate = Builder.CreateExtractElement(Mask, Idx);
     }
 
     // Create "cond" block
     //

 // cond.load1:
 // %Ptr1 = extractelement <16 x i32*> %Ptrs, i32 1
 // %Load1 = load i32, i32* %Ptr1, align 4
 // %Res1 = insertelement <16 x i32> %res.phi.else, i32 %Load1, i32 1
 // br label %else2
 // . . .
 // %Result = select <16 x i1> %Mask, <16 x i32> %res.phi.select, <16 x i32> %Src
 // ret <16 x i32> %Result
-static void scalarizeMaskedGather(CallInst *CI, bool &ModifiedDT) {
+static void scalarizeMaskedGather(const DataLayout &DL, CallInst *CI,
+                                  bool &ModifiedDT) {
   Value *Ptrs = CI->getArgOperand(0);
   Value *Alignment = CI->getArgOperand(1);
   Value *Mask = CI->getArgOperand(2);
   Value *Src0 = CI->getArgOperand(3);
 
   auto *VecType = cast<FixedVectorType>(CI->getType());
   Type *EltTy = VecType->getElementType();
 
for (unsigned Idx = 0; Idx < VectorWidth; ++Idx) {
     //
     //  %Mask1 = and i16 %scalar_mask, i32 1 << Idx
     //  %cond = icmp ne i16 %mask_1, 0
     //  br i1 %Mask1, label %cond.load, label %else
     //
 
     Value *Predicate;
     if (VectorWidth != 1) {
-      Value *Mask = Builder.getInt(APInt::getOneBitSet(VectorWidth, Idx));
+      Value *Mask = Builder.getInt(APInt::getOneBitSet(
+          VectorWidth, adjustForEndian(DL, VectorWidth, Idx)));
       Predicate = Builder.CreateICmpNE(Builder.CreateAnd(SclrMask, Mask),
                                        Builder.getIntN(VectorWidth, 0));
     } else {
       Predicate = Builder.CreateExtractElement(Mask, Idx, "Mask" + Twine(Idx));
     }
 
     // Create "cond" block
     //

 // br i1 %Mask1, label %cond.store1, label %else2
 //
 // cond.store1:
 // %Elt1 = extractelement <16 x i32> %Src, i32 1
 // %Ptr1 = extractelement <16 x i32*> %Ptrs, i32 1
 // store i32 %Elt1, i32* %Ptr1, align 4
 // br label %else2
 //   . . .
-static void scalarizeMaskedScatter(CallInst *CI, bool &ModifiedDT) {
+static void scalarizeMaskedScatter(const DataLayout &DL, CallInst *CI,
+                                   bool &ModifiedDT) {
   Value *Src = CI->getArgOperand(0);
   Value *Ptrs = CI->getArgOperand(1);
   Value *Alignment = CI->getArgOperand(2);
   Value *Mask = CI->getArgOperand(3);
 
   auto *SrcFVTy = cast<FixedVectorType>(Src->getType());
 
   assert(
for (unsigned Idx = 0; Idx < VectorWidth; ++Idx) {
     // Fill the "else" block, created in the previous iteration
     //
     //  %Mask1 = and i16 %scalar_mask, i32 1 << Idx
     //  %cond = icmp ne i16 %mask_1, 0
     //  br i1 %Mask1, label %cond.store, label %else
     //
     Value *Predicate;
     if (VectorWidth != 1) {
-      Value *Mask = Builder.getInt(APInt::getOneBitSet(VectorWidth, Idx));
+      Value *Mask = Builder.getInt(APInt::getOneBitSet(
+          VectorWidth, adjustForEndian(DL, VectorWidth, Idx)));
       Predicate = Builder.CreateICmpNE(Builder.CreateAnd(SclrMask, Mask),
                                        Builder.getIntN(VectorWidth, 0));
     } else {
       Predicate = Builder.CreateExtractElement(Mask, Idx, "Mask" + Twine(Idx));
     }
 
     // Create "cond" block
     //
for (unsigned Idx = 0; Idx < VectorWidth; ++Idx) {
     OldBr->eraseFromParent();
     IfBlock = NewIfBlock;
   }
   CI->eraseFromParent();
 
   ModifiedDT = true;
 }
 
-static void scalarizeMaskedExpandLoad(CallInst *CI, bool &ModifiedDT) {
+static void scalarizeMaskedExpandLoad(const DataLayout &DL, CallInst *CI,
+                                      bool &ModifiedDT) {
   Value *Ptr = CI->getArgOperand(0);
   Value *Mask = CI->getArgOperand(1);
   Value *PassThru = CI->getArgOperand(2);
 
   auto *VecType = cast<FixedVectorType>(CI->getType());
 
   Type *EltTy = VecType->getElementType();
 
for (unsigned Idx = 0; Idx < VectorWidth; ++Idx) {
     //
     //  %res.phi.else3 = phi <16 x i32> [ %11, %cond.load1 ], [ %res.phi.else, %else ]
     //  %mask_1 = extractelement <16 x i1> %mask, i32 Idx
     //  br i1 %mask_1, label %cond.load, label %else
     //
 
     Value *Predicate;
     if (VectorWidth != 1) {
-      Value *Mask = Builder.getInt(APInt::getOneBitSet(VectorWidth, Idx));
+      Value *Mask = Builder.getInt(APInt::getOneBitSet(
+          VectorWidth, adjustForEndian(DL, VectorWidth, Idx)));
       Predicate = Builder.CreateICmpNE(Builder.CreateAnd(SclrMask, Mask),
                                        Builder.getIntN(VectorWidth, 0));
     } else {
       Predicate = Builder.CreateExtractElement(Mask, Idx, "Mask" + Twine(Idx));
     }
 
     // Create "cond" block
     //
static void scalarizeMaskedExpandLoad(const DataLayout &DL, CallInst *CI,
   }
 
   CI->replaceAllUsesWith(VResult);
   CI->eraseFromParent();
 
   ModifiedDT = true;
 }
 
-static void scalarizeMaskedCompressStore(CallInst *CI, bool &ModifiedDT) {
+static void scalarizeMaskedCompressStore(const DataLayout &DL, CallInst *CI,
+                                         bool &ModifiedDT) {
   Value *Src = CI->getArgOperand(0);
   Value *Ptr = CI->getArgOperand(1);
   Value *Mask = CI->getArgOperand(2);
 
   auto *VecType = cast<FixedVectorType>(Src->getType());
 
   IRBuilder<> Builder(CI->getContext());
   Instruction *InsertPt = CI;
static void scalarizeMaskedCompressStore(const DataLayout &DL, CallInst *CI,
   for (unsigned Idx = 0; Idx < VectorWidth; ++Idx) {
     // Fill the "else" block, created in the previous iteration
     //
     //  %mask_1 = extractelement <16 x i1> %mask, i32 Idx
     //  br i1 %mask_1, label %cond.store, label %else
     //
     Value *Predicate;
     if (VectorWidth != 1) {
-      Value *Mask = Builder.getInt(APInt::getOneBitSet(VectorWidth, Idx));
+      Value *Mask = Builder.getInt(APInt::getOneBitSet(
+          VectorWidth, adjustForEndian(DL, VectorWidth, Idx)));
       Predicate = Builder.CreateICmpNE(Builder.CreateAnd(SclrMask, Mask),
                                        Builder.getIntN(VectorWidth, 0));
     } else {
       Predicate = Builder.CreateExtractElement(Mask, Idx, "Mask" + Twine(Idx));
     }
 
     // Create "cond" block
     //
if (II) {
     default:
       break;
     case Intrinsic::masked_load:
       // Scalarize unsupported vector masked load
       if (TTI.isLegalMaskedLoad(
               CI->getType(),
               cast<ConstantInt>(CI->getArgOperand(1))->getAlignValue()))
         return false;
-      scalarizeMaskedLoad(CI, ModifiedDT);
+      scalarizeMaskedLoad(DL, CI, ModifiedDT);
       return true;
     case Intrinsic::masked_store:
       if (TTI.isLegalMaskedStore(
               CI->getArgOperand(0)->getType(),
               cast<ConstantInt>(CI->getArgOperand(2))->getAlignValue()))
         return false;
-      scalarizeMaskedStore(CI, ModifiedDT);
+      scalarizeMaskedStore(DL, CI, ModifiedDT);
       return true;
     case Intrinsic::masked_gather: {
       unsigned AlignmentInt =
           cast<ConstantInt>(CI->getArgOperand(1))->getZExtValue();
       Type *LoadTy = CI->getType();
       Align Alignment =
           DL.getValueOrABITypeAlignment(MaybeAlign(AlignmentInt), LoadTy);
       if (TTI.isLegalMaskedGather(LoadTy, Alignment))
         return false;
-      scalarizeMaskedGather(CI, ModifiedDT);
+      scalarizeMaskedGather(DL, CI, ModifiedDT);
       return true;
     }
     case Intrinsic::masked_scatter: {
       unsigned AlignmentInt =
           cast<ConstantInt>(CI->getArgOperand(2))->getZExtValue();
       Type *StoreTy = CI->getArgOperand(0)->getType();
       Align Alignment =
           DL.getValueOrABITypeAlignment(MaybeAlign(AlignmentInt), StoreTy);
       if (TTI.isLegalMaskedScatter(StoreTy, Alignment))
         return false;
-      scalarizeMaskedScatter(CI, ModifiedDT);
+      scalarizeMaskedScatter(DL, CI, ModifiedDT);
       return true;
     }
     case Intrinsic::masked_expandload:
       if (TTI.isLegalMaskedExpandLoad(CI->getType()))
         return false;
-      scalarizeMaskedExpandLoad(CI, ModifiedDT);
+      scalarizeMaskedExpandLoad(DL, CI, ModifiedDT);
       return true;
     case Intrinsic::masked_compressstore:
       if (TTI.isLegalMaskedCompressStore(CI->getArgOperand(0)->getType()))
         return false;
-      scalarizeMaskedCompressStore(CI, ModifiedDT);
+      scalarizeMaskedCompressStore(DL, CI, ModifiedDT);
       return true;
     }
   }
 
   return false;
 }