[ScalarizeMaskedMemIntrin] Use the element type to calculate alignment for gather/scatter when alignment operand is 0.

[craig.topper]

Summary

Previously we used the vector type, but we're loading/storing
invididual elements so I think only element alignment should matter.

Noticed while looking at the code for something else so I don't
have a test case.

[lebedev.ri]

I think this is correct.
I guess for that you need to pass 0 into alignment argument of these intrinsics.

[craig.topper]

In D105220#2850946, @lebedev.ri wrote:

I think this is correct.
I guess for that you need to pass 0 into alignment argument of these intrinsics.

Yeah. And to have an observable change you need a target with a cost model that returns a different answer if you give it element alignment versus vector alignment. But since gather works on elements, I would expect that any cost model that cares about alignment is checking whether it is element aligned or not.

[efriedma]

Instead of messing with this, can we just change the verifier to require that the alignment argument to masked_gather is nonzero? (And teach autoupgrade to fix old code.) This is just a lesser version of the same mess we used to have with other load/store operations.

If you're going to fix it this way, though, please also fix SelectionDAGBuilder.

[craig.topper]

In D105220#2851174, @efriedma wrote:

Instead of messing with this, can we just change the verifier to require that the alignment argument to masked_gather is nonzero? (And teach autoupgrade to fix old code.) This is just a lesser version of the same mess we used to have with other load/store operations.

If you're going to fix it this way, though, please also fix SelectionDAGBuilder.

I stared at the autoupgrade code for intrinsics. I can't find any cases of autoupgrading an intrinsic by changing the value of an argument. It looks like the interface with the LLParser and BitcodeReader don't quite work for this. I think we need to return true with NewFn = nullptr from UpgradeIntrinsicFunction. That will enable UpgradeCallsToIntrinsic to call UpgradeIntrinsicCall which will enable LLParser. BitCodeReader calls UpgradeIntrinsicCall directly. Then in UpgradeIntrinsicCall we would need to modify the argument and generate a new call with the correct arguments. But I think UpgradeCallsToIntrinsic or the BitCodeReader will then try to delete the intrinsic declaration after all calls are upgraded because they don't expect AutoUpgrade to replace a call with a call to the same intrinsic. They think the declaration should be dead after upgrading all the calls.

[craig.topper]

Update SelectionDAGBuilder as well.

[efriedma]

LGTM

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

void SelectionDAGBuilder::visitMaskedScatter(const CallInst &I) {
 
   // llvm.masked.scatter.*(Src0, Ptrs, alignment, Mask)
   const Value *Ptr = I.getArgOperand(1);
   SDValue Src0 = getValue(I.getArgOperand(0));
   SDValue Mask = getValue(I.getArgOperand(3));
   EVT VT = Src0.getValueType();
   Align Alignment = cast<ConstantInt>(I.getArgOperand(2))
                         ->getMaybeAlignValue()
-                        .getValueOr(DAG.getEVTAlign(VT));
+                        .getValueOr(DAG.getEVTAlign(VT.getScalarType()));
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
 
   AAMDNodes AAInfo;
   I.getAAMetadata(AAInfo);
 
   SDValue Base;
   SDValue Index;
   ISD::MemIndexType IndexType;
void SelectionDAGBuilder::visitMaskedGather(const CallInst &I) {
   const Value *Ptr = I.getArgOperand(0);
   SDValue Src0 = getValue(I.getArgOperand(3));
   SDValue Mask = getValue(I.getArgOperand(2));
 
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
   EVT VT = TLI.getValueType(DAG.getDataLayout(), I.getType());
   Align Alignment = cast<ConstantInt>(I.getArgOperand(1))
                         ->getMaybeAlignValue()
-                        .getValueOr(DAG.getEVTAlign(VT));
+                        .getValueOr(DAG.getEVTAlign(VT.getScalarType()));
 
   AAMDNodes AAInfo;
   I.getAAMetadata(AAInfo);
   const MDNode *Ranges = I.getMetadata(LLVMContext::MD_range);
 
   SDValue Root = DAG.getRoot();
   SDValue Base;
   SDValue Index;

llvm/lib/Transforms/Scalar/ScalarizeMaskedMemIntrin.cpp

if (II) {
     case Intrinsic::masked_store:
       if (TTI.isLegalMaskedStore(
               CI->getArgOperand(0)->getType(),
               cast<ConstantInt>(CI->getArgOperand(2))->getAlignValue()))
         return false;
       scalarizeMaskedStore(DL, CI, DTU, ModifiedDT);
       return true;
     case Intrinsic::masked_gather: {
-      unsigned AlignmentInt =
-          cast<ConstantInt>(CI->getArgOperand(1))->getZExtValue();
+      MaybeAlign MA =
+          cast<ConstantInt>(CI->getArgOperand(1))->getMaybeAlignValue();
       Type *LoadTy = CI->getType();
-      Align Alignment =
-          DL.getValueOrABITypeAlignment(MaybeAlign(AlignmentInt), LoadTy);
+      Align Alignment = DL.getValueOrABITypeAlignment(MA,
+                                                      LoadTy->getScalarType());
       if (TTI.isLegalMaskedGather(LoadTy, Alignment))
         return false;
       scalarizeMaskedGather(DL, CI, DTU, ModifiedDT);
       return true;
     }
     case Intrinsic::masked_scatter: {
-      unsigned AlignmentInt =
-          cast<ConstantInt>(CI->getArgOperand(2))->getZExtValue();
+      MaybeAlign MA =
+          cast<ConstantInt>(CI->getArgOperand(2))->getMaybeAlignValue();
       Type *StoreTy = CI->getArgOperand(0)->getType();
-      Align Alignment =
-          DL.getValueOrABITypeAlignment(MaybeAlign(AlignmentInt), StoreTy);
+      Align Alignment = DL.getValueOrABITypeAlignment(MA,
+                                                      StoreTy->getScalarType());
       if (TTI.isLegalMaskedScatter(StoreTy, Alignment))
         return false;
       scalarizeMaskedScatter(DL, CI, DTU, ModifiedDT);
       return true;
     }
     case Intrinsic::masked_expandload:
       if (TTI.isLegalMaskedExpandLoad(CI->getType()))
         return false;