[LV][VPlan] Extract the implementation of transform Recipe to EVLRecipe into a small function. NFC #119510
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LiqinWeng
requested review from
fhahn and
Mel-Chen
and removed request for
fhahn
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-vectorizers Author: LiqinWeng (LiqinWeng) ChangesFull diff: https://github.com/llvm/llvm-project/pull/119510.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index 922cba7831f4e9..7436949b28335f 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -1438,13 +1438,100 @@ void VPlanTransforms::addActiveLaneMask(
HeaderMask->replaceAllUsesWith(LaneMask);
}
+static VPRecipeBase *createEVLRecipe(VPValue &EVL, VPValue *HeaderMask,
+ VPValue *AllOneMask,
+ VPRecipeBase *CurRecipe,
+ VPTypeAnalysis TypeInfo) {
+ using namespace llvm::VPlanPatternMatch;
+ auto GetNewMask = [&](VPValue *OrigMask) -> VPValue * {
+ assert(OrigMask && "Unmasked recipe when folding tail");
+ return HeaderMask == OrigMask ? nullptr : OrigMask;
+ };
+
+ return TypeSwitch<VPRecipeBase *, VPRecipeBase *>(CurRecipe)
+ .Case<VPWidenLoadRecipe>([&](VPWidenLoadRecipe *L) {
+ VPValue *NewMask = GetNewMask(L->getMask());
+ return new VPWidenLoadEVLRecipe(*L, EVL, NewMask);
+ })
+ .Case<VPWidenStoreRecipe>([&](VPWidenStoreRecipe *S) {
+ VPValue *NewMask = GetNewMask(S->getMask());
+ return new VPWidenStoreEVLRecipe(*S, EVL, NewMask);
+ })
+ .Case<VPWidenRecipe>([&](VPWidenRecipe *W) -> VPRecipeBase * {
+ unsigned Opcode = W->getOpcode();
+ if (!Instruction::isBinaryOp(Opcode) && !Instruction::isUnaryOp(Opcode))
+ return nullptr;
+ return new VPWidenEVLRecipe(*W, EVL);
+ })
+ .Case<VPReductionRecipe>([&](VPReductionRecipe *Red) {
+ VPValue *NewMask = GetNewMask(Red->getCondOp());
+ return new VPReductionEVLRecipe(*Red, EVL, NewMask);
+ })
+ .Case<VPWidenIntrinsicRecipe>(
+ [&](VPWidenIntrinsicRecipe *CInst) -> VPRecipeBase * {
+ auto *CI = cast<CallInst>(CInst->getUnderlyingInstr());
+ Intrinsic::ID VPID = VPIntrinsic::getForIntrinsic(
+ CI->getCalledFunction()->getIntrinsicID());
+ assert(VPID != Intrinsic::not_intrinsic &&
+ "Expected VP Instrinsic");
+
+ SmallVector<VPValue *> Ops(CInst->operands());
+ assert(VPIntrinsic::getMaskParamPos(VPID) &&
+ VPIntrinsic::getVectorLengthParamPos(VPID) &&
+ "Expected VP intrinsic");
+
+ Ops.push_back(AllOneMask);
+ Ops.push_back(&EVL);
+ return new VPWidenIntrinsicRecipe(*CI, VPID, Ops,
+ TypeInfo.inferScalarType(CInst),
+ CInst->getDebugLoc());
+ })
+ .Case<VPWidenCastRecipe>([&](VPWidenCastRecipe *CInst) -> VPRecipeBase * {
+ auto *CI = dyn_cast<CastInst>(CInst->getUnderlyingInstr());
+ Intrinsic::ID VPID = VPIntrinsic::getForOpcode(CI->getOpcode());
+ assert(VPID != Intrinsic::not_intrinsic &&
+ "Expected vp.casts Instrinsic");
+
+ SmallVector<VPValue *> Ops(CInst->operands());
+ assert(VPIntrinsic::getMaskParamPos(VPID) &&
+ VPIntrinsic::getVectorLengthParamPos(VPID) &&
+ "Expected VP intrinsic");
+ Ops.push_back(AllOneMask);
+ Ops.push_back(&EVL);
+ return new VPWidenIntrinsicRecipe(
+ VPID, Ops, TypeInfo.inferScalarType(CInst), CInst->getDebugLoc());
+ })
+ .Case<VPWidenSelectRecipe>([&](VPWidenSelectRecipe *Sel) {
+ SmallVector<VPValue *> Ops(Sel->operands());
+ Ops.push_back(&EVL);
+ return new VPWidenIntrinsicRecipe(Intrinsic::vp_select, Ops,
+ TypeInfo.inferScalarType(Sel),
+ Sel->getDebugLoc());
+ })
+ .Case<VPInstruction>([&](VPInstruction *VPI) -> VPRecipeBase * {
+ VPValue *LHS, *RHS;
+ // Transform select with a header mask condition
+ // select(header_mask, LHS, RHS)
+ // into vector predication merge.
+ // vp.merge(all-true, LHS, RHS, EVL)
+ if (!match(VPI, m_Select(m_Specific(HeaderMask), m_VPValue(LHS),
+ m_VPValue(RHS))))
+ return nullptr;
+ // Use all true as the condition because this transformation is
+ // limited to selects whose condition is a header mask.
+ return new VPWidenIntrinsicRecipe(
+ Intrinsic::vp_merge, {AllOneMask, LHS, RHS, &EVL},
+ TypeInfo.inferScalarType(LHS), VPI->getDebugLoc());
+ })
+ .Default([&](VPRecipeBase *R) { return nullptr; });
+}
+
/// Replace recipes with their EVL variants.
static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) {
- using namespace llvm::VPlanPatternMatch;
Type *CanonicalIVType = Plan.getCanonicalIV()->getScalarType();
VPTypeAnalysis TypeInfo(CanonicalIVType);
LLVMContext &Ctx = CanonicalIVType->getContext();
- SmallVector<VPValue *> HeaderMasks = collectAllHeaderMasks(Plan);
+ VPValue *AllOneMask = Plan.getOrAddLiveIn(ConstantInt::getTrue(Ctx));
for (VPUser *U : Plan.getVF().users()) {
if (auto *R = dyn_cast<VPReverseVectorPointerRecipe>(U))
@@ -1454,112 +1541,23 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) {
for (VPValue *HeaderMask : collectAllHeaderMasks(Plan)) {
for (VPUser *U : collectUsersRecursively(HeaderMask)) {
auto *CurRecipe = cast<VPRecipeBase>(U);
- auto GetNewMask = [&](VPValue *OrigMask) -> VPValue * {
- assert(OrigMask && "Unmasked recipe when folding tail");
- return HeaderMask == OrigMask ? nullptr : OrigMask;
- };
-
- VPRecipeBase *NewRecipe =
- TypeSwitch<VPRecipeBase *, VPRecipeBase *>(CurRecipe)
- .Case<VPWidenLoadRecipe>([&](VPWidenLoadRecipe *L) {
- VPValue *NewMask = GetNewMask(L->getMask());
- return new VPWidenLoadEVLRecipe(*L, EVL, NewMask);
- })
- .Case<VPWidenStoreRecipe>([&](VPWidenStoreRecipe *S) {
- VPValue *NewMask = GetNewMask(S->getMask());
- return new VPWidenStoreEVLRecipe(*S, EVL, NewMask);
- })
- .Case<VPWidenRecipe>([&](VPWidenRecipe *W) -> VPRecipeBase * {
- unsigned Opcode = W->getOpcode();
- if (!Instruction::isBinaryOp(Opcode) &&
- !Instruction::isUnaryOp(Opcode))
- return nullptr;
- return new VPWidenEVLRecipe(*W, EVL);
- })
- .Case<VPReductionRecipe>([&](VPReductionRecipe *Red) {
- VPValue *NewMask = GetNewMask(Red->getCondOp());
- return new VPReductionEVLRecipe(*Red, EVL, NewMask);
- })
- .Case<VPWidenIntrinsicRecipe>(
- [&](VPWidenIntrinsicRecipe *CInst) -> VPRecipeBase * {
- auto *CI = cast<CallInst>(CInst->getUnderlyingInstr());
- Intrinsic::ID VPID = VPIntrinsic::getForIntrinsic(
- CI->getCalledFunction()->getIntrinsicID());
- if (VPID == Intrinsic::not_intrinsic)
- return nullptr;
-
- SmallVector<VPValue *> Ops(CInst->operands());
- assert(VPIntrinsic::getMaskParamPos(VPID) &&
- VPIntrinsic::getVectorLengthParamPos(VPID) &&
- "Expected VP intrinsic");
- VPValue *Mask = Plan.getOrAddLiveIn(ConstantInt::getTrue(
- IntegerType::getInt1Ty(CI->getContext())));
- Ops.push_back(Mask);
- Ops.push_back(&EVL);
- return new VPWidenIntrinsicRecipe(
- *CI, VPID, Ops, TypeInfo.inferScalarType(CInst),
- CInst->getDebugLoc());
- })
- .Case<VPWidenCastRecipe>(
- [&](VPWidenCastRecipe *CInst) -> VPRecipeBase * {
- auto *CI = dyn_cast<CastInst>(CInst->getUnderlyingInstr());
- Intrinsic::ID VPID =
- VPIntrinsic::getForOpcode(CI->getOpcode());
- assert(VPID != Intrinsic::not_intrinsic &&
- "Expected vp.casts Instrinsic");
-
- SmallVector<VPValue *> Ops(CInst->operands());
- assert(VPIntrinsic::getMaskParamPos(VPID) &&
- VPIntrinsic::getVectorLengthParamPos(VPID) &&
- "Expected VP intrinsic");
- VPValue *Mask = Plan.getOrAddLiveIn(ConstantInt::getTrue(
- IntegerType::getInt1Ty(CI->getContext())));
- Ops.push_back(Mask);
- Ops.push_back(&EVL);
- return new VPWidenIntrinsicRecipe(
- VPID, Ops, TypeInfo.inferScalarType(CInst),
- CInst->getDebugLoc());
- })
- .Case<VPWidenSelectRecipe>([&](VPWidenSelectRecipe *Sel) {
- SmallVector<VPValue *> Ops(Sel->operands());
- Ops.push_back(&EVL);
- return new VPWidenIntrinsicRecipe(Intrinsic::vp_select, Ops,
- TypeInfo.inferScalarType(Sel),
- Sel->getDebugLoc());
- })
- .Case<VPInstruction>([&](VPInstruction *VPI) -> VPRecipeBase * {
- VPValue *LHS, *RHS;
- // Transform select with a header mask condition
- // select(header_mask, LHS, RHS)
- // into vector predication merge.
- // vp.merge(all-true, LHS, RHS, EVL)
- if (!match(VPI, m_Select(m_Specific(HeaderMask), m_VPValue(LHS),
- m_VPValue(RHS))))
- return nullptr;
- // Use all true as the condition because this transformation is
- // limited to selects whose condition is a header mask.
- VPValue *AllTrue =
- Plan.getOrAddLiveIn(ConstantInt::getTrue(Ctx));
- return new VPWidenIntrinsicRecipe(
- Intrinsic::vp_merge, {AllTrue, LHS, RHS, &EVL},
- TypeInfo.inferScalarType(LHS), VPI->getDebugLoc());
- })
- .Default([&](VPRecipeBase *R) { return nullptr; });
-
- if (!NewRecipe)
+
+ VPRecipeBase *EVLRecipe =
+ createEVLRecipe(EVL, HeaderMask, AllOneMask, CurRecipe, TypeInfo);
+ if (!EVLRecipe)
continue;
- [[maybe_unused]] unsigned NumDefVal = NewRecipe->getNumDefinedValues();
+ [[maybe_unused]] unsigned NumDefVal = EVLRecipe->getNumDefinedValues();
assert(NumDefVal == CurRecipe->getNumDefinedValues() &&
"New recipe must define the same number of values as the "
"original.");
assert(
NumDefVal <= 1 &&
"Only supports recipes with a single definition or without users.");
- NewRecipe->insertBefore(CurRecipe);
- if (isa<VPSingleDefRecipe, VPWidenLoadEVLRecipe>(NewRecipe)) {
+ EVLRecipe->insertBefore(CurRecipe);
+ if (isa<VPSingleDefRecipe, VPWidenLoadEVLRecipe>(EVLRecipe)) {
VPValue *CurVPV = CurRecipe->getVPSingleValue();
- CurVPV->replaceAllUsesWith(NewRecipe->getVPSingleValue());
+ CurVPV->replaceAllUsesWith(EVLRecipe->getVPSingleValue());
}
CurRecipe->eraseFromParent();
}
|
LiqinWeng
force-pushed
the
refactor-vplantranstoevlrecipe
branch
from
8777eb0 to
7df9397
Compare
| static VPRecipeBase *createEVLRecipe(VPValue &EVL, VPValue *HeaderMask, | ||
| VPValue *AllOneMask, | ||
| VPRecipeBase *CurRecipe, | ||
| VPTypeAnalysis TypeInfo) { |
There was a problem hiding this comment.
VPTypeAnalysis &TypeInfo.
Also, if EVL is passed by ref, beter all other params pass by ref
fhahn
reviewed
Dec 11, 2024
| @@ -1438,13 +1438,100 @@ void VPlanTransforms::addActiveLaneMask( | |||
| HeaderMask->replaceAllUsesWith(LaneMask); | |||
| } | |||
|
|
|||
| static VPRecipeBase *createEVLRecipe(VPValue &EVL, VPValue *HeaderMask, | |||
Comment on lines
1496
to
1492
| assert(VPIntrinsic::getMaskParamPos(VPID) && | ||
| VPIntrinsic::getVectorLengthParamPos(VPID) && | ||
| "Expected VP intrinsic"); |
There was a problem hiding this comment.
Meant move next to other assert above
Mel-Chen
reviewed
Dec 12, 2024
| @@ -1438,13 +1438,98 @@ void VPlanTransforms::addActiveLaneMask( | |||
| HeaderMask->replaceAllUsesWith(LaneMask); | |||
| } | |||
|
|
|||
| /// Create EVLRecipe with Recipe | |||
There was a problem hiding this comment.
The comment needs to be more detailed.
Due to the numerous arguments, it might be helpful to use \p in the comment to reference arguments for further explanation. The comments for getPredicatedMask and mergeReplicateRegionsIntoSuccessors in the same file serve as great references.
| Type *CanonicalIVType = Plan.getCanonicalIV()->getScalarType(); | ||
| VPTypeAnalysis TypeInfo(CanonicalIVType); | ||
| LLVMContext &Ctx = CanonicalIVType->getContext(); | ||
| SmallVector<VPValue *> HeaderMasks = collectAllHeaderMasks(Plan); |
There was a problem hiding this comment.
Thanks for removing this redundant line. :)
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
LiqinWeng
force-pushed
the
refactor-vplantranstoevlrecipe
branch
from
aa1d1c8 to
f4bc49a
Compare
fhahn
reviewed
Dec 13, 2024
| assert(VPID != Intrinsic::not_intrinsic && | ||
| "Expected vp.casts Instrinsic"); | ||
|
|
||
| SmallVector<VPValue *> Ops(CInst->operands()); | ||
| assert(VPIntrinsic::getMaskParamPos(VPID) && | ||
| VPIntrinsic::getVectorLengthParamPos(VPID) && | ||
| "Expected VP intrinsic"); |
There was a problem hiding this comment.
Suggested change
| assert(VPID != Intrinsic::not_intrinsic && | |
| "Expected vp.casts Instrinsic"); | |
| SmallVector<VPValue *> Ops(CInst->operands()); | |
| assert(VPIntrinsic::getMaskParamPos(VPID) && | |
| VPIntrinsic::getVectorLengthParamPos(VPID) && | |
| "Expected VP intrinsic"); | |
| assert(VPID != Intrinsic::not_intrinsic && | |
| "Expected vp.casts Instrinsic"); | |
| assert(VPIntrinsic::getMaskParamPos(VPID) && | |
| VPIntrinsic::getVectorLengthParamPos(VPID) && | |
| "Expected VP intrinsic"); | |
| SmallVector<VPValue *> Ops(CInst->operands()); |
Comment on lines
1496
to
1492
| assert(VPIntrinsic::getMaskParamPos(VPID) && | ||
| VPIntrinsic::getVectorLengthParamPos(VPID) && | ||
| "Expected VP intrinsic"); |
There was a problem hiding this comment.
Meant move next to other assert above
fhahn
reviewed
Dec 13, 2024
| @@ -1438,13 +1438,105 @@ void VPlanTransforms::addActiveLaneMask( | |||
| HeaderMask->replaceAllUsesWith(LaneMask); | |||
| } | |||
|
|
|||
| // Convert each widen Recipe to a widen EVLRecipe in VectorLoopRegion. | |||
There was a problem hiding this comment.
Suggested change
| // Convert each widen Recipe to a widen EVLRecipe in VectorLoopRegion. | |
| // Try to convert \p CurRecipe to a corresponding EVL-based recipe. Returns nullptr if no EVL-based recipe could be created. |
| CInst->getDebugLoc()); | ||
| }) | ||
| .Case<VPWidenCastRecipe>([&](VPWidenCastRecipe *CInst) -> VPRecipeBase * { | ||
| auto *CI = dyn_cast<CastInst>(CInst->getUnderlyingInstr()); |
There was a problem hiding this comment.
Just noticing this here, but this should use the opcode from the recipe, not from the underlying instruction.
Also is using dyn_cast ,but never checking if the result is non-null?
Can we share most of the logic/asserts with the VPWidenIntrinsicRecipe case?
| }) | ||
| .Case<VPWidenIntrinsicRecipe>( | ||
| [&](VPWidenIntrinsicRecipe *CInst) -> VPRecipeBase * { | ||
| auto *CI = dyn_cast<CallInst>(CInst->getUnderlyingInstr()); |
There was a problem hiding this comment.
Just noticing this here, but this should use the intrinsic ID from the recipe, not from the underlying call.
Might be good to fix first separately.
There was a problem hiding this comment.
There is currently no test case to cover it，do you have any way to construct a test for this scenario?
LiqinWeng
force-pushed
the
refactor-vplantranstoevlrecipe
branch
from
3bc3f19 to
7460da9
Compare
LiqinWeng
force-pushed
the
refactor-vplantranstoevlrecipe
branch
2 times, most recently
from
9a10d48 to
8efbbd0
Compare
|
Have any other problems？:) @fhahn @Mel-Chen @alexey-bataev |
| @@ -1727,7 +1727,7 @@ define float @fmuladd(ptr %a, ptr %b, i64 %n, float %start) { | |||
| ; IF-EVL-NEXT: [[TMP14:%.*]] = getelementptr inbounds float, ptr [[B:%.*]], i64 [[TMP11]] | |||
| ; IF-EVL-NEXT: [[TMP15:%.*]] = getelementptr inbounds float, ptr [[TMP14]], i32 0 | |||
| ; IF-EVL-NEXT: [[VP_OP_LOAD1:%.*]] = call <vscale x 4 x float> @llvm.vp.load.nxv4f32.p0(ptr align 4 [[TMP15]], <vscale x 4 x i1> splat (i1 true), i32 [[TMP10]]) | |||
| ; IF-EVL-NEXT: [[TMP16:%.*]] = call reassoc <vscale x 4 x float> @llvm.vp.fmuladd.nxv4f32(<vscale x 4 x float> [[VP_OP_LOAD]], <vscale x 4 x float> [[VP_OP_LOAD1]], <vscale x 4 x float> [[VEC_PHI]], <vscale x 4 x i1> splat (i1 true), i32 [[TMP10]]) | |||
There was a problem hiding this comment.
I think we should preserve flags after EVL transformation.
Perhaps add/modify a constructor of VPWidenInstrinsicRecipe with flags independent from underlying instruction.
There was a problem hiding this comment.
I have addressed this in another patch. pls review： #119847
There was a problem hiding this comment.
This is changing because we aren't passing the underlying instruction to the VPWIdenIntrinsic constructor any longer, right?
I think it is fine to change it, but the patch isn't NFC because of it. Maybe keep passing the underlying instruction in this patch, and then change the constructor separately to keep this just a NFC refactoring?
There was a problem hiding this comment.
I create the new patch to deal with it，pls ： #120816
fhahn
reviewed
Dec 20, 2024
| @@ -1727,7 +1727,7 @@ define float @fmuladd(ptr %a, ptr %b, i64 %n, float %start) { | |||
| ; IF-EVL-NEXT: [[TMP14:%.*]] = getelementptr inbounds float, ptr [[B:%.*]], i64 [[TMP11]] | |||
| ; IF-EVL-NEXT: [[TMP15:%.*]] = getelementptr inbounds float, ptr [[TMP14]], i32 0 | |||
| ; IF-EVL-NEXT: [[VP_OP_LOAD1:%.*]] = call <vscale x 4 x float> @llvm.vp.load.nxv4f32.p0(ptr align 4 [[TMP15]], <vscale x 4 x i1> splat (i1 true), i32 [[TMP10]]) | |||
| ; IF-EVL-NEXT: [[TMP16:%.*]] = call reassoc <vscale x 4 x float> @llvm.vp.fmuladd.nxv4f32(<vscale x 4 x float> [[VP_OP_LOAD]], <vscale x 4 x float> [[VP_OP_LOAD1]], <vscale x 4 x float> [[VEC_PHI]], <vscale x 4 x i1> splat (i1 true), i32 [[TMP10]]) | |||
There was a problem hiding this comment.
This is changing because we aren't passing the underlying instruction to the VPWIdenIntrinsic constructor any longer, right?
I think it is fine to change it, but the patch isn't NFC because of it. Maybe keep passing the underlying instruction in this patch, and then change the constructor separately to keep this just a NFC refactoring?
LiqinWeng
force-pushed
the
refactor-vplantranstoevlrecipe
branch
from
8efbbd0 to
d1279c2
Compare
fhahn
approved these changes
Dec 21, 2024
| @@ -1442,13 +1442,93 @@ void VPlanTransforms::addActiveLaneMask( | |||
| HeaderMask->replaceAllUsesWith(LaneMask); | |||
| } | |||
|
|
|||
| // Try to convert \p CurRecipe to a corresponding EVL-based recipe. Returns | |||
There was a problem hiding this comment.
Suggested change
| // Try to convert \p CurRecipe to a corresponding EVL-based recipe. Returns | |
| /// Try to convert \p CurRecipe to a corresponding EVL-based recipe. Returns |
and below
…pe into a small function
LiqinWeng
force-pushed
the
refactor-vplantranstoevlrecipe
branch
from
d1279c2 to
9c2d84d
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/4003 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/6577 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/8145 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/186/builds/5102 Here is the relevant piece of the build log for the reference |
LiqinWeng
added a commit
to LiqinWeng/llvm-project
that referenced
this pull request
Dec 23, 2024
Resolve the compilation error caused by the merge issue: llvm#119510
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/6873 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.