[llvm] 8ba2cbf - [NFC] Remove dead code
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 22 07:43:36 PDT 2022
Er, I think this needs reverted, at minimum reapplied with a better
comment, and probably reviewed.
A couple problems here:
1) This is not "dead code" in any definition I'm aware of. This is
inlining a named helper function to make it dead, and then deleting it.
As such, the submission comment is misleading and may cause interesting
parties to miss it.
2) I see clarity value in the original helper name. This is somewhat
debatable, and I wouldn't treat this as blocking if someone else
knowledgeable of the code was fine with the removal. Thus the review.
Philip
On 6/22/22 06:34, Guillaume Chatelet via llvm-commits wrote:
> Author: Guillaume Chatelet
> Date: 2022-06-22T13:33:58Z
> New Revision: 8ba2cbff70f2c49a8926451c59cc260d67b706cf
>
> URL: https://github.com/llvm/llvm-project/commit/8ba2cbff70f2c49a8926451c59cc260d67b706cf
> DIFF: https://github.com/llvm/llvm-project/commit/8ba2cbff70f2c49a8926451c59cc260d67b706cf.diff
>
> LOG: [NFC] Remove dead code
>
> Added:
>
>
> Modified:
> llvm/include/llvm/Support/Alignment.h
> llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
> llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
> llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
> llvm/unittests/Support/AlignmentTest.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/llvm/include/llvm/Support/Alignment.h b/llvm/include/llvm/Support/Alignment.h
> index 0b1d52061f2c..11e574b600a3 100644
> --- a/llvm/include/llvm/Support/Alignment.h
> +++ b/llvm/include/llvm/Support/Alignment.h
> @@ -208,10 +208,6 @@ inline uint64_t offsetToAlignedAddr(const void *Addr, Align Alignment) {
> /// Returns the log2 of the alignment.
> inline unsigned Log2(Align A) { return A.ShiftValue; }
>
> -/// Returns the alignment that satisfies both alignments.
> -/// Same semantic as MinAlign.
> -inline Align commonAlignment(Align A, Align B) { return std::min(A, B); }
> -
> /// Returns the alignment that satisfies both alignments.
> /// Same semantic as MinAlign.
> inline Align commonAlignment(Align A, uint64_t Offset) {
>
> diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
> index bd100a297504..fb046d519ac8 100644
> --- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
> +++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
> @@ -7627,7 +7627,7 @@ LegalizerHelper::lowerMemcpy(MachineInstr &MI, Register Dst, Register Src,
>
> bool DstAlignCanChange = false;
> MachineFrameInfo &MFI = MF.getFrameInfo();
> - Align Alignment = commonAlignment(DstAlign, SrcAlign);
> + Align Alignment = std::min(DstAlign, SrcAlign);
>
> MachineInstr *FIDef = getOpcodeDef(TargetOpcode::G_FRAME_INDEX, Dst, MRI);
> if (FIDef && !MFI.isFixedObjectIndex(FIDef->getOperand(1).getIndex()))
> @@ -7735,7 +7735,7 @@ LegalizerHelper::lowerMemmove(MachineInstr &MI, Register Dst, Register Src,
> bool DstAlignCanChange = false;
> MachineFrameInfo &MFI = MF.getFrameInfo();
> bool OptSize = shouldLowerMemFuncForSize(MF);
> - Align Alignment = commonAlignment(DstAlign, SrcAlign);
> + Align Alignment = std::min(DstAlign, SrcAlign);
>
> MachineInstr *FIDef = getOpcodeDef(TargetOpcode::G_FRAME_INDEX, Dst, MRI);
> if (FIDef && !MFI.isFixedObjectIndex(FIDef->getOperand(1).getIndex()))
>
> diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> index fc031ce824d2..6dfc774d043f 100644
> --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> @@ -5863,7 +5863,7 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
> // @llvm.memcpy defines 0 and 1 to both mean no alignment.
> Align DstAlign = MCI.getDestAlign().valueOrOne();
> Align SrcAlign = MCI.getSourceAlign().valueOrOne();
> - Align Alignment = commonAlignment(DstAlign, SrcAlign);
> + Align Alignment = std::min(DstAlign, SrcAlign);
> bool isVol = MCI.isVolatile();
> bool isTC = I.isTailCall() && isInTailCallPosition(I, DAG.getTarget());
> // FIXME: Support passing
> diff erent dest/src alignments to the memcpy DAG
> @@ -5886,7 +5886,7 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
> // @llvm.memcpy.inline defines 0 and 1 to both mean no alignment.
> Align DstAlign = MCI.getDestAlign().valueOrOne();
> Align SrcAlign = MCI.getSourceAlign().valueOrOne();
> - Align Alignment = commonAlignment(DstAlign, SrcAlign);
> + Align Alignment = std::min(DstAlign, SrcAlign);
> bool isVol = MCI.isVolatile();
> bool isTC = I.isTailCall() && isInTailCallPosition(I, DAG.getTarget());
> // FIXME: Support passing
> diff erent dest/src alignments to the memcpy DAG
> @@ -5941,7 +5941,7 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
> // @llvm.memmove defines 0 and 1 to both mean no alignment.
> Align DstAlign = MMI.getDestAlign().valueOrOne();
> Align SrcAlign = MMI.getSourceAlign().valueOrOne();
> - Align Alignment = commonAlignment(DstAlign, SrcAlign);
> + Align Alignment = std::min(DstAlign, SrcAlign);
> bool isVol = MMI.isVolatile();
> bool isTC = I.isTailCall() && isInTailCallPosition(I, DAG.getTarget());
> // FIXME: Support passing
> diff erent dest/src alignments to the memmove DAG
>
> diff --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
> index d87c8d77fffd..1f5bc69acecd 100644
> --- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
> +++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
> @@ -773,7 +773,7 @@ bool MemCpyOptPass::processStore(StoreInst *SI, BasicBlock::iterator &BBI) {
> LI, SI, SI->getPointerOperand()->stripPointerCasts(),
> LI->getPointerOperand()->stripPointerCasts(),
> DL.getTypeStoreSize(SI->getOperand(0)->getType()),
> - commonAlignment(SI->getAlign(), LI->getAlign()), GetCall);
> + std::min(SI->getAlign(), LI->getAlign()), GetCall);
> if (changed) {
> eraseInstruction(SI);
> eraseInstruction(LI);
>
> diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
> index 631dbb35f450..2e5a150debc6 100644
> --- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
> +++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
> @@ -3558,7 +3558,7 @@ static LoadsState canVectorizeLoads(ArrayRef<Value *> VL, const Value *VL0,
> Align CommonAlignment = cast<LoadInst>(VL0)->getAlign();
> for (Value *V : VL)
> CommonAlignment =
> - commonAlignment(CommonAlignment, cast<LoadInst>(V)->getAlign());
> + std::min(CommonAlignment, cast<LoadInst>(V)->getAlign());
> auto *VecTy = FixedVectorType::get(ScalarTy, VL.size());
> if (TTI.isLegalMaskedGather(VecTy, CommonAlignment) &&
> !TTI.forceScalarizeMaskedGather(VecTy, CommonAlignment))
> @@ -6438,7 +6438,7 @@ InstructionCost BoUpSLP::getEntryCost(const TreeEntry *E,
> Align CommonAlignment = Alignment;
> for (Value *V : VL)
> CommonAlignment =
> - commonAlignment(CommonAlignment, cast<LoadInst>(V)->getAlign());
> + std::min(CommonAlignment, cast<LoadInst>(V)->getAlign());
> VecLdCost = TTI->getGatherScatterOpCost(
> Instruction::Load, VecTy, cast<LoadInst>(VL0)->getPointerOperand(),
> /*VariableMask=*/false, CommonAlignment, CostKind, VL0);
> @@ -8150,7 +8150,7 @@ Value *BoUpSLP::vectorizeTree(TreeEntry *E) {
> Align CommonAlignment = LI->getAlign();
> for (Value *V : E->Scalars)
> CommonAlignment =
> - commonAlignment(CommonAlignment, cast<LoadInst>(V)->getAlign());
> + std::min(CommonAlignment, cast<LoadInst>(V)->getAlign());
> NewLI = Builder.CreateMaskedGather(VecTy, VecPtr, CommonAlignment);
> }
> Value *V = propagateMetadata(NewLI, E->Scalars);
>
> diff --git a/llvm/unittests/Support/AlignmentTest.cpp b/llvm/unittests/Support/AlignmentTest.cpp
> index 0514db28a243..0baec54b3380 100644
> --- a/llvm/unittests/Support/AlignmentTest.cpp
> +++ b/llvm/unittests/Support/AlignmentTest.cpp
> @@ -115,21 +115,6 @@ TEST(AlignmentTest, Log2) {
> }
> }
>
> -TEST(AlignmentTest, MinAlign) {
> - struct {
> - uint64_t A;
> - uint64_t B;
> - uint64_t MinAlign;
> - } kTests[] = {
> - {1, 2, 1},
> - {8, 4, 4},
> - };
> - for (const auto &T : kTests) {
> - EXPECT_EQ(MinAlign(T.A, T.B), T.MinAlign);
> - EXPECT_EQ(commonAlignment(Align(T.A), Align(T.B)), T.MinAlign);
> - }
> -}
> -
> TEST(AlignmentTest, Encode_Decode) {
> for (uint64_t Value : getValidAlignments()) {
> {
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list