[IR] masked gather/scatter alignment should be set

[gchatelet]

Summary

masked_load and masked_store instructions require the alignment to be specified and a power of two. It seems to me that this requirement applies to masked_gather and masked_scatter as well.

[delena]

llvm/test/CodeGen/X86/avx2-masked-gather.ll

54 ↗

(On Diff #239553)

I think 0 is also good. You can put alignment 0 in load/store.

[gchatelet]

llvm/test/CodeGen/X86/avx2-masked-gather.ll

54 ↗

(On Diff #239553)

It seems like it's not the case for masked load / store
unittest
godbolt

Alignment needs to be a non 0 power of two.

Any reasons why we would want it to be 0?

[gchatelet]

llvm/test/CodeGen/X86/avx2-masked-gather.ll

54 ↗

(On Diff #239553)

Looking at the source code it seems like masked_gather depends on an aligned_load which should have a non zero alignment?

[delena]

CreateAlignedLoad allows align 0, as far as I see in the code.
We just assume that masked.gather is created from scalar load by vectorizer. Vectorizer just copies alignment from scalar load to masked load/gather and the requirement to alignment should match load alignment.

[gchatelet]

In D73179#1835730, @delena wrote:

CreateAlignedLoad allows align 0, as far as I see in the code.
We just assume that masked.gather is created from scalar load by vectorizer. Vectorizer just copies alignment from scalar load to masked load/gather and the requirement to alignment should match load alignment.

I see, then you're write that 0 should be allowed.
I'm trying to tighten the alignment type throughout LLVM so I always try to have the most restrictive type.
I'll change the Patch accordingly.

[gchatelet]

• Address comments

[gchatelet]

• Fixing error message

[gchatelet]

Any other comments on you side @delena ?

[gchatelet]

Thx !

llvm/docs/LangRef.rst

 """""""""
 
 Reads scalar values from arbitrary memory locations and gathers them into one vector. The memory locations are provided in the vector of pointers '``ptrs``'. The memory is accessed according to the provided mask. The mask holds a bit for each vector lane, and is used to prevent memory accesses to the masked-off lanes. The masked-off lanes in the result vector are taken from the corresponding lanes of the '``passthru``' operand.
 
 
 Arguments:
 """"""""""
 
-The first operand is a vector of pointers which holds all memory addresses to read. The second operand is an alignment of the source addresses. It must be a constant integer value. The third operand, mask, is a vector of boolean values with the same number of elements as the return type. The fourth is a pass-through value that is used to fill the masked-off lanes of the result. The return type, underlying type of the vector of pointers and the type of the '``passthru``' operand are the same vector types.
-
+The first operand is a vector of pointers which holds all memory addresses to read. The second operand is an alignment of the source addresses. It must be 0 or a power of two constant integer value. The third operand, mask, is a vector of boolean values with the same number of elements as the return type. The fourth is a pass-through value that is used to fill the masked-off lanes of the result. The return type, underlying type of the vector of pointers and the type of the '``passthru``' operand are the same vector types.
 
 Semantics:
 """"""""""
 
 The '``llvm.masked.gather``' intrinsic is designed for conditional reading of multiple scalar values from arbitrary memory locations in a single IR operation. It is useful for targets that support vector masked gathers and allows vectorizing basic blocks with data and control divergence. Other targets may support this intrinsic differently, for example by lowering it into a sequence of scalar load operations.
 The semantics of this operation are equivalent to a sequence of conditional scalar loads with subsequent gathering all loaded values into a single vector. The mask restricts memory access to certain lanes and facilitates vectorization of predicated basic blocks.
 
 

 Overview:
 """""""""
 
 Writes each element from the value vector to the corresponding memory address. The memory addresses are represented as a vector of pointers. Writing is done according to the provided mask. The mask holds a bit for each vector lane, and is used to prevent memory accesses to the masked-off lanes.
 
 Arguments:
 """"""""""
 
-The first operand is a vector value to be written to memory. The second operand is a vector of pointers, pointing to where the value elements should be stored. It has the same underlying type as the value operand. The third operand is an alignment of the destination addresses. The fourth operand, mask, is a vector of boolean values. The types of the mask and the value operand must have the same number of vector elements.
-
+The first operand is a vector value to be written to memory. The second operand is a vector of pointers, pointing to where the value elements should be stored. It has the same underlying type as the value operand. The third operand is an alignment of the destination addresses. It must be 0 or a power of two constant integer value. The fourth operand, mask, is a vector of boolean values. The types of the mask and the value operand must have the same number of vector elements.
 
 Semantics:
 """"""""""
 
 The '``llvm.masked.scatter``' intrinsics is designed for writing selected vector elements to arbitrary memory addresses in a single IR operation. The operation may be conditional, when not all bits in the mask are switched on. It is useful for targets that support vector masked scatter and allows vectorizing basic blocks with data and control divergence. Other targets may support this intrinsic differently, for example by lowering it into a sequence of branches that guard scalar store operations.
 
 ::
 

llvm/lib/IR/Verifier.cpp

case Intrinsic::masked_store: {
     Assert(DataTy == Val->getType(),
            "masked_store: storee must match pointer type", Call);
     Assert(Mask->getType()->getVectorNumElements() ==
                DataTy->getVectorNumElements(),
            "masked_store: vector mask must be same length as data", Call);
     break;
   }
 
+  case Intrinsic::masked_gather: {
+    const APInt &Alignment =
+        cast<ConstantInt>(Call.getArgOperand(1))->getValue();
+    Assert(Alignment.isNullValue() || Alignment.isPowerOf2(),
+           "masked_gather: alignment must be 0 or a power of 2", Call);
+    break;
+  }
+  case Intrinsic::masked_scatter: {
+    const APInt &Alignment =
+        cast<ConstantInt>(Call.getArgOperand(2))->getValue();
+    Assert(Alignment.isNullValue() || Alignment.isPowerOf2(),
+           "masked_scatter: alignment must be 0 or a power of 2", Call);
+    break;
+  }
+
   case Intrinsic::experimental_guard: {
     Assert(isa<CallInst>(Call), "experimental_guard cannot be invoked", Call);
     Assert(Call.countOperandBundlesOfType(LLVMContext::OB_deopt) == 1,
            "experimental_guard must have exactly one "
            "\"deopt\" operand bundle");
     break;
   }
 

llvm/test/Assembler/auto_upgrade_intrinsics.ll

; CHECK: @llvm.masked.gather.v2f64.v2p0f64
   ret <2 x double> %res
 }
 
 declare void @llvm.masked.scatter.v2f64(<2 x double> %val, <2 x double*> %ptrs, i32, <2 x i1> %mask)
 
 define void @tests.masked.scatter(<2 x double*> %ptr, <2 x i1> %mask, <2 x double> %val)  {
 ; CHECK-LABEL: @tests.masked.scatter(
 ; CHECK: @llvm.masked.scatter.v2f64.v2p0f64
-  call void @llvm.masked.scatter.v2f64(<2 x double> %val, <2 x double*> %ptr, i32 3, <2 x i1> %mask)
+  call void @llvm.masked.scatter.v2f64(<2 x double> %val, <2 x double*> %ptr, i32 1, <2 x i1> %mask)
   ret void
 }
 
 declare {}* @llvm.invariant.start(i64, i8* nocapture) nounwind readonly
 declare void @llvm.invariant.end({}*, i64, i8* nocapture) nounwind
 
 define void @tests.invariant.start.end() {
   ; CHECK-LABEL: @tests.invariant.start.end(

llvm/test/Transforms/InstCombine/masked_intrinsics.ll

;
   ret <2 x double> %res
 }
 
 
 define void @scatter_zeromask(<2 x double*> %ptrs, <2 x double> %val)  {
 ; CHECK-LABEL: @scatter_zeromask(
 ; CHECK-NEXT:    ret void
 ;
-  call void @llvm.masked.scatter.v2f64.v2p0f64(<2 x double> %val, <2 x double*> %ptrs, i32 6, <2 x i1> zeroinitializer)
+  call void @llvm.masked.scatter.v2f64.v2p0f64(<2 x double> %val, <2 x double*> %ptrs, i32 8, <2 x i1> zeroinitializer)
   ret void
 }
 
 define void @scatter_demandedelts(double* %ptr, double %val)  {
 ; CHECK-LABEL: @scatter_demandedelts(
 ; CHECK-NEXT:    [[PTRS:%.*]] = getelementptr double, double* [[PTR:%.*]], <2 x i64> <i64 0, i64 undef>
 ; CHECK-NEXT:    [[VALVEC2:%.*]] = insertelement <2 x double> undef, double [[VAL:%.*]], i32 0
 ; CHECK-NEXT:    call void @llvm.masked.scatter.v2f64.v2p0f64(<2 x double> [[VALVEC2]], <2 x double*> [[PTRS]], i32 8, <2 x i1> <i1 true, i1 false>)
 ; CHECK-NEXT:    ret void
 ;
   %ptrs = getelementptr double, double* %ptr, <2 x i64> <i64 0, i64 1>
   %valvec1 = insertelement <2 x double> undef, double %val, i32 0
   %valvec2 = insertelement <2 x double> %valvec1, double %val, i32 1
   call void @llvm.masked.scatter.v2f64.v2p0f64(<2 x double> %valvec2, <2 x double*> %ptrs, i32 8, <2 x i1> <i1 true, i1 false>)
   ret void
 }