[NFC][Clang][RISCV] Reduce boilerplate when determining prototype for segment loads

[eopXD]

Summary

No functionality change of the RVV builtin and compiler intrinsics is intended
in this patch.

This patch gathers scattered comments for the segment load builtin/intrinsics
and its variants (e.g. segment unit-stride load, segment strided load) into a
single paragraph under riscv_vector.td.

This patch also tries to reduce one level of the if-statements as the push_back
are essentially the same actions but differs in index based on the the value
of the policy attributes and whether the intrinsic is masked.

[eopXD]

Rebase to latest main.

[khchen]

clang/include/clang/Basic/riscv_vector.td

928

After remove the builtins comment I don't have idea what's going on this piece of code. but I'm okay if there is no concern from the other reviewers.

930–942

nit: nit: there is no assert now, is it still NFC? I'm not sure.

[craig.topper]

clang/include/clang/Basic/riscv_vector.td

943

This doesn't look right to me. This append call appends PassThruOperand NF times. In the non-agnostic case, it needs to append NF different values.

[eopXD]

Update code based on landing of D140678.

The motivation of this NFC patch comes from the struggle while I was tracing through them. I think the scattered comments of boilerplates with only something slightly different makes the code hard to read, that is why I am gathering the comments together into a sole paragraph. The multi-level if-statement has duplicated actions and gets me confused from time-to-time.

[khchen]

LGTM. Thanks for clean up code!

clang/include/clang/Basic/riscv_vector.td

820

nit: there is no unmasked builtin comment here.

1068

In fact, VLOperand is always coming from Ops.back()

[eopXD]

Update comment section for segment load.

[eopXD]

Update comment section: remove mis-indented indents.

[kito-cheng]

Go ahead, thanks for clean up!

clang/include/clang/Basic/riscv_vector.td

let HasMaskedOffOperand = false,
             def : RVVBuiltin<"Uv", "0PUe" # eew64_type # "UvUv", type>;
           }
         }
       }
   }
 }
 
 defvar NFList = [2, 3, 4, 5, 6, 7, 8];
+/*
+A segment load builtin has different variants.
+
+Therefore a segment unit-stride load builtin can have 4 variants,
+1. When unmasked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Ptr, VL)
+2. When masked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Mask, Ptr, VL)
+3. When unmasked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, VL)
+4. When masked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Mask, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, VL)
+
+Other variants of segment load builtin share the same structure, but they
+have their own extra parameter.
+
+The segment unit-stride fault-only-first load builtin has a 'NewVL'
+operand after the 'Ptr' operand.
+1. When unmasked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Ptr, NewVL, VL)
+2. When masked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Mask, Ptr, NewVL, VL)
+3. When unmasked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, NewVL, VL)
+4. When masked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Mask, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, NewVL, VL)
+
+The segment strided load builtin has a 'Stride' operand after the 'Ptr'
+operand.
+1. When unmasked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Ptr, Stride, VL)
+2. When masked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Mask, Ptr, Stride, VL)
+3. When unmasked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, Stride, VL)
+4. When masked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Mask, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, Stride, VL)
+
+The segment indexed load builtin has a 'Idx' operand after the 'Ptr' operand.
+1. When unmasked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Ptr, Idx, VL)
+2. When masked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Mask, Ptr, Idx, VL)
+3. When unmasked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, Idx, VL)
+4. When masked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Mask, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, Idx, VL)
+
+Segment load intrinsics has different variants similar to their builtins.
+
+Segment unit-stride load intrinsic,
+  Masked: (Vector0, ..., Vector{NF - 1}, Ptr, Mask, VL, Policy)
+  Unmasked: (Vector0, ..., Vector{NF - 1}, Ptr, VL)
+Segment unit-stride fault-only-first load intrinsic,
+  Masked: (Vector0, ..., Vector{NF - 1}, Ptr, Mask, VL, Policy)
+  Unmasked: (Vector0, ..., Vector{NF - 1}, Ptr, VL)
+Segment strided load intrinsic,
+  Masked: (Vector0, ..., Vector{NF - 1}, Ptr, Stride, Mask, VL, Policy)
+  Unmasked: (Vector0, ..., Vector{NF - 1}, Ptr, Stride, VL)
+Segment indexed load intrinsic,
+  Masked: (Vector0, ..., Vector{NF - 1}, Ptr, Index, Mask, VL, Policy)
+  Unmasked: (Vector0, ..., Vector{NF - 1}, Ptr, Index, VL)
+
+The Vector(s) is poison when the policy behavior allows us to not care
+about any masked-off elements.
+*/
 
 class PVString<int nf, bit signed> {
   string S =
     !cond(!eq(nf, 2): !if(signed, "PvPv", "PUvPUv"),
           !eq(nf, 3): !if(signed, "PvPvPv", "PUvPUvPUv"),
           !eq(nf, 4): !if(signed, "PvPvPvPv", "PUvPUvPUvPUv"),
           !eq(nf, 5): !if(signed, "PvPvPvPvPv", "PUvPUvPUvPUvPUv"),
           !eq(nf, 6): !if(signed, "PvPvPvPvPvPv", "PUvPUvPUvPUvPUvPUv"),
defvar eew = !cond(!eq(type, "c") : "8",
         let Name = op # nf # "e" # eew # "_v",
             IRName = op # nf,
             MaskedIRName = op # nf # "_mask",
             NF = nf,
             ManualCodegen = [{
     {
       ResultType = ConvertType(E->getArg(0)->getType()->getPointeeType());
       IntrinsicTypes = {ResultType, Ops.back()->getType()};
-      SmallVector<llvm::Value*, 10> Operands;
-      if (IsMasked) {
-        // TAMA builtin: (val0 address, ..., mask, ptr, vl)
-        // builtin: (val0 address, ..., mask, maskedoff0, ..., ptr, vl)
-        // intrinsic: (maskedoff0, ..., ptr, mask, vl)
-        if (PolicyAttrs == TAIL_AGNOSTIC_MASK_AGNOSTIC) {
-          Operands.append(NF, llvm::PoisonValue::get(ResultType));
-          Operands.push_back(Ops[NF + 1]);
-          Operands.push_back(Ops[NF]);
-          Operands.push_back(Ops[NF + 2]);
-        } else {
+      SmallVector<llvm::Value*, 12> Operands;
+
+      // Please refer to comment under 'defvar NFList' in this file
+      if ((IsMasked && PolicyAttrs == TAIL_AGNOSTIC_MASK_AGNOSTIC) ||
+          (!IsMasked && PolicyAttrs == TAIL_AGNOSTIC))
+      else {
+        if (IsMasked)
           Operands.append(Ops.begin() + NF + 1, Ops.begin() + 2 * NF + 1);
-          Operands.push_back(Ops[2 * NF + 1]);
-          Operands.push_back(Ops[NF]);
-          Operands.push_back(Ops[2 * NF + 2]);
-        }
-        Operands.push_back(ConstantInt::get(Ops.back()->getType(), PolicyAttrs));
-        assert(Operands.size() == NF + 4);
-      } else {
-        // TA builtin: (val0 address, val1 address, ..., ptr, vl)
-        // TU builtin: (val0 address, ..., passthru0, ..., ptr, vl)
-        // intrinsic: (passthru0, passthru1, ..., ptr, vl)
-        if (PolicyAttrs == TAIL_AGNOSTIC) {
-          Operands.append(NF, llvm::PoisonValue::get(ResultType));
-          Operands.push_back(Ops[NF]);
-          Operands.push_back(Ops[NF + 1]);
-        } else {
-          Operands.append(Ops.begin() + NF, Ops.begin() + 2 * NF + 2);
-        }
-      }
+        else // Unmasked
+          Operands.append(Ops.begin() + NF, Ops.begin() + 2 * NF);
+      unsigned PtrOperandIdx = IsMasked ?
+        (PolicyAttrs == TAIL_AGNOSTIC_MASK_AGNOSTIC) ? NF + 1 : 2 * NF + 1 :
+        (PolicyAttrs == TAIL_AGNOSTIC) ? NF : 2 * NF;
+      Value *PtrOperand = Ops[PtrOperandIdx];
+      Value *VLOperand = Ops[PtrOperandIdx + 1];
+      Operands.push_back(PtrOperand);
+      if (IsMasked)
+      Operands.push_back(VLOperand);
+      if (IsMasked)
+        Operands.push_back(ConstantInt::get(Ops.back()->getType(), PolicyAttrs));
+
       llvm::Function *F = CGM.getIntrinsic(ID, IntrinsicTypes);
       llvm::Value *LoadValue = Builder.CreateCall(F, Operands, "");
       clang::CharUnits Align =
           CGM.getNaturalPointeeTypeAlignment(E->getArg(0)->getType());
       llvm::Value *V;
       for (unsigned I = 0; I < NF; ++I) {
         llvm::Value *Val = Builder.CreateExtractValue(LoadValue, {I});
         V = Builder.CreateStore(Val, Address(Ops[I], Val->getType(), Align));
       }
defvar eew = !cond(!eq(type, "c") : "8",
             IRName = op # nf # "ff",
             MaskedIRName = op # nf # "ff_mask",
             NF = nf,
             ManualCodegen = [{
     {
       ResultType = ConvertType(E->getArg(0)->getType()->getPointeeType());
       IntrinsicTypes = {ResultType, Ops.back()->getType()};
       SmallVector<llvm::Value*, 12> Operands;
-      Value *NewVL;
 
-      if (IsMasked) {
-        // TAMA builtin: (val0 address, ..., mask, ptr, new_vl, vl)
-        // builtin: (val0 address, ..., mask, maskedoff0, ..., ptr, new_vl, vl)
-        // intrinsic: (maskedoff0, ..., ptr, mask, vl)
-        if (PolicyAttrs == TAIL_AGNOSTIC_MASK_AGNOSTIC) {
-          Operands.append(NF, llvm::PoisonValue::get(ResultType));
-          Operands.push_back(Ops[NF + 1]);
-          Operands.push_back(Ops[NF]);
-          Operands.push_back(Ops[NF + 3]);
-          NewVL = Ops[NF + 2];
-        } else {
+      // Please refer to comment under 'defvar NFList' in this file
+      if ((IsMasked && PolicyAttrs == TAIL_AGNOSTIC_MASK_AGNOSTIC) ||
+          (!IsMasked && PolicyAttrs == TAIL_AGNOSTIC))
+      else {
+        if (IsMasked)
           Operands.append(Ops.begin() + NF + 1, Ops.begin() + 2 * NF + 1);
-          Operands.push_back(Ops[2 * NF + 1]);
-          Operands.push_back(Ops[NF]);
-          Operands.push_back(Ops[2 * NF + 3]);
-          NewVL = Ops[2 * NF + 2];
-        }
-        Operands.push_back(ConstantInt::get(Ops.back()->getType(), PolicyAttrs));
-        assert(Operands.size() == NF + 4);
-      } else {
-        // TA builtin: (val0 address, val1 address, ..., ptr, new_vl, vl)
-        // TU builtin: (val0 address, ..., passthru0, ..., ptr, new_vl, vl)
-        // intrinsic: (passthru0, passthru1, ..., ptr, vl)
-        if (PolicyAttrs == TAIL_AGNOSTIC) {
-          Operands.append(NF, llvm::PoisonValue::get(ResultType));
-          Operands.push_back(Ops[NF]);
-          Operands.push_back(Ops[NF + 2]);
-          NewVL = Ops[NF + 1];
-        } else {
+        else // Unmasked
           Operands.append(Ops.begin() + NF, Ops.begin() + 2 * NF);
-          Operands.push_back(Ops[2 * NF]);
-          Operands.push_back(Ops[2 * NF + 2]);
-          NewVL = Ops[2 * NF + 1];
-        }
       }
+      unsigned PtrOperandIdx = IsMasked ?
+        (PolicyAttrs == TAIL_AGNOSTIC_MASK_AGNOSTIC) ? NF + 1 : 2 * NF + 1 :
+        (PolicyAttrs == TAIL_AGNOSTIC) ? NF : 2 * NF;
+      Value *PtrOperand = Ops[PtrOperandIdx];
+      Value *NewVLOperand = Ops[PtrOperandIdx + 1];
+      Value *VLOperand = Ops[PtrOperandIdx + 2];
+      Operands.push_back(PtrOperand);
+      if (IsMasked)
+        Operands.push_back(Ops[NF]);
+      Operands.push_back(VLOperand);
+      if (IsMasked)
+        Operands.push_back(ConstantInt::get(Ops.back()->getType(), PolicyAttrs));
+
       llvm::Function *F = CGM.getIntrinsic(ID, IntrinsicTypes);
       llvm::Value *LoadValue = Builder.CreateCall(F, Operands, "");
       clang::CharUnits Align =
           CGM.getNaturalPointeeTypeAlignment(E->getArg(0)->getType());
       for (unsigned I = 0; I < NF; ++I) {
         llvm::Value *Val = Builder.CreateExtractValue(LoadValue, {I});
         Builder.CreateStore(Val, Address(Ops[I], Val->getType(), Align));
       }
       // Store new_vl.
       llvm::Value *Val = Builder.CreateExtractValue(LoadValue, {NF});
-      return Builder.CreateStore(Val, Address(NewVL, Val->getType(), Align));
+      return Builder.CreateStore(Val, Address(NewVLOperand, Val->getType(), Align));
     }
     }] in {
           defvar PV = PVString<nf, /*signed=*/true>.S;
           defvar PUV = PVString<nf, /*signed=*/false>.S;
           def : RVVBuiltin<"v", "0" # PV # "PCe" # "Pz", type>;
           if !not(IsFloat<type>.val) then {
             def : RVVBuiltin<"Uv", "0" # PUV # "PCUe" # "Pz", type>;
         }
defvar eew = !cond(!eq(type, "c") : "8",
             MaskedIRName = op # nf # "_mask",
             NF = nf,
             ManualCodegen = [{
     {
       ResultType = ConvertType(E->getArg(0)->getType()->getPointeeType());
       IntrinsicTypes = {ResultType, Ops.back()->getType()};
       SmallVector<llvm::Value*, 12> Operands;
 
-      if (IsMasked) {
-        // TAMA builtin: (val0 address, ..., mask, ptr, stride, vl)
-        // builtin: (val0 address, ..., mask, maskedoff0, ..., ptr, stride, vl)
-        // intrinsic: (maskedoff0, ..., ptr, stride, mask, vl)
-        if (PolicyAttrs == TAIL_AGNOSTIC_MASK_AGNOSTIC) {
-          Operands.append(NF, llvm::PoisonValue::get(ResultType));
-          Operands.push_back(Ops[NF + 1]);
-          Operands.push_back(Ops[NF + 2]);
-          Operands.push_back(Ops[NF]);
-          Operands.push_back(Ops[NF + 3]);
-        } else {
+      // Please refer to comment under 'defvar NFList' in this file
+      if ((IsMasked && PolicyAttrs == TAIL_AGNOSTIC_MASK_AGNOSTIC) ||
+          (!IsMasked && PolicyAttrs == TAIL_AGNOSTIC))
+      else {
+        if (IsMasked)
           Operands.append(Ops.begin() + NF + 1, Ops.begin() + 2 * NF + 1);
-          Operands.push_back(Ops[2 * NF + 1]);
-          Operands.push_back(Ops[2 * NF + 2]);
-          Operands.push_back(Ops[NF]);
-          Operands.push_back(Ops[2 * NF + 3]);
-        }
-        Operands.push_back(ConstantInt::get(Ops.back()->getType(), PolicyAttrs));
-        assert(Operands.size() == NF + 5);
-      } else {
-        // TA builtin: (val0 address, val1 address, ..., ptr, stride, vl)
-        // TU builtin: (val0 address, ..., passthru0, ..., ptr, stride, vl)
-        // intrinsic: (passthru0, passthru1, ..., ptr, stride, vl)
-        if (PolicyAttrs == TAIL_AGNOSTIC) {
-          Operands.append(NF, llvm::PoisonValue::get(ResultType));
-          Operands.push_back(Ops[NF]);
-          Operands.push_back(Ops[NF + 1]);
-          Operands.push_back(Ops[NF + 2]);
-        } else {
+        else // Unmasked
           Operands.append(Ops.begin() + NF, Ops.begin() + 2 * NF);
-          Operands.push_back(Ops[2 * NF]);
-          Operands.push_back(Ops[2 * NF + 1]);
-          Operands.push_back(Ops[2 * NF + 2]);
-        }
       }
+      unsigned PtrOperandIdx = IsMasked ?
+        (PolicyAttrs == TAIL_AGNOSTIC_MASK_AGNOSTIC) ? NF + 1 : 2 * NF + 1 :
+        (PolicyAttrs == TAIL_AGNOSTIC) ? NF : 2 * NF;
+      Value *PtrOperand = Ops[PtrOperandIdx];
+      Value *StrideOperand = Ops[PtrOperandIdx + 1];
+      Value *VLOperand = Ops[PtrOperandIdx + 2];
+      Operands.push_back(PtrOperand);
+      Operands.push_back(StrideOperand);
+      if (IsMasked)
+        Operands.push_back(Ops[NF]);
+      Operands.push_back(VLOperand);
+      if (IsMasked)
+        Operands.push_back(ConstantInt::get(Ops.back()->getType(), PolicyAttrs));
+
       llvm::Function *F = CGM.getIntrinsic(ID, IntrinsicTypes);
       llvm::Value *LoadValue = Builder.CreateCall(F, Operands, "");
       clang::CharUnits Align =
           CGM.getNaturalPointeeTypeAlignment(E->getArg(0)->getType());
       llvm::Value *V;
       for (unsigned I = 0; I < NF; ++I) {
         llvm::Value *Val = Builder.CreateExtractValue(LoadValue, {I});
         V = Builder.CreateStore(Val, Address(Ops[I], Val->getType(), Align));
foreach eew_info = EEWList in {
         let Name = op # nf # "ei" # eew # "_v",
             IRName = op # nf,
             MaskedIRName = op # nf # "_mask",
             NF = nf,
             ManualCodegen = [{
     {
       ResultType = ConvertType(E->getArg(0)->getType()->getPointeeType());
       SmallVector<llvm::Value*, 12> Operands;
-      if (IsMasked) {
-        // TAMA builtin: (val0 address, ..., mask, ptr, index, vl)
-        // builtin: (val0 address, ..., mask, maskedoff0, ..., ptr, index, vl)
-        // intrinsic: (maskedoff0, ..., ptr, index, mask, vl)
-        if (PolicyAttrs == TAIL_AGNOSTIC_MASK_AGNOSTIC) {
-          Operands.append(NF, llvm::PoisonValue::get(ResultType));
-          Operands.push_back(Ops[NF + 1]);
-          Operands.push_back(Ops[NF + 2]);
-          Operands.push_back(Ops[NF]);
-          Operands.push_back(Ops[NF + 3]);
-          IntrinsicTypes = {ResultType, Ops[NF + 2]->getType(), Ops.back()->getType()};
-        } else {
+
+      // Please refer to comment under 'defvar NFList' in this file
+      if ((IsMasked && PolicyAttrs == TAIL_AGNOSTIC_MASK_AGNOSTIC) ||
+          (!IsMasked && PolicyAttrs == TAIL_AGNOSTIC))
+      else {
+        if (IsMasked)
           Operands.append(Ops.begin() + NF + 1, Ops.begin() + 2 * NF + 1);
-          Operands.push_back(Ops[2 * NF + 1]);
-          Operands.push_back(Ops[2 * NF + 2]);
-          Operands.push_back(Ops[NF]);
-          Operands.push_back(Ops[2 * NF + 3]);
-          IntrinsicTypes = {ResultType, Ops[2 * NF + 2]->getType(), Ops.back()->getType()};
-        }
-        Operands.push_back(ConstantInt::get(Ops.back()->getType(), PolicyAttrs));
-        assert(Operands.size() == NF + 5);
-      } else {
-        // TA builtin: (val0 address, val1 address, ..., ptr, index, vl)
-        // TU builtin: (val0 address, ..., passthru0, ..., ptr, index, vl)
-        // intrinsic: (passthru0, passthru1, ..., ptr, index, vl)
-        if (PolicyAttrs == TAIL_AGNOSTIC) {
-          Operands.append(NF, llvm::PoisonValue::get(ResultType));
-          Operands.push_back(Ops[NF]);
-          Operands.push_back(Ops[NF + 1]);
-          Operands.push_back(Ops[NF + 2]);
-          IntrinsicTypes = {ResultType, Ops[NF + 1]->getType(), Ops.back()->getType()};
-        } else {
+        else // Unmasked
           Operands.append(Ops.begin() + NF, Ops.begin() + 2 * NF);
-          Operands.push_back(Ops[2 * NF]);
-          Operands.push_back(Ops[2 * NF + 1]);
-          Operands.push_back(Ops[2 * NF + 2]);
-          IntrinsicTypes = {ResultType, Ops[2 * NF + 1]->getType(), Ops.back()->getType()};
-        }
       }
+      unsigned PtrOperandIdx = IsMasked ?
+        (PolicyAttrs == TAIL_AGNOSTIC_MASK_AGNOSTIC) ? NF + 1 : 2 * NF + 1 :
+        (PolicyAttrs == TAIL_AGNOSTIC) ? NF : 2 * NF;
+      Value *PtrOperand = Ops[PtrOperandIdx];
+      Value *IndexOperand = Ops[PtrOperandIdx + 1];
+      Value *VLOperand = Ops[PtrOperandIdx + 2];
+      Operands.push_back(PtrOperand);
+      Operands.push_back(IndexOperand);
+      if (IsMasked)
+        Operands.push_back(Ops[NF]);
+      Operands.push_back(VLOperand);
+      if (IsMasked)
+        Operands.push_back(ConstantInt::get(Ops.back()->getType(), PolicyAttrs));
+      IntrinsicTypes = {ResultType, IndexOperand->getType(), Ops.back()->getType()};
+
       llvm::Function *F = CGM.getIntrinsic(ID, IntrinsicTypes);
       llvm::Value *LoadValue = Builder.CreateCall(F, Operands, "");
       clang::CharUnits Align =
           CGM.getNaturalPointeeTypeAlignment(E->getArg(0)->getType());
       llvm::Value *V;
       for (unsigned I = 0; I < NF; ++I) {
         llvm::Value *Val = Builder.CreateExtractValue(LoadValue, {I});
         V = Builder.CreateStore(Val, Address(Ops[I], Val->getType(), Align));