Skip to content

Commit

Permalink
[ArgPromotion][Attributor] Update min-legal-vector-width when do prom…
Browse files Browse the repository at this point in the history
…otion

X86 codegen uses function attribute `min-legal-vector-width` to select the proper ABI. The intention of the attribute is to reflect user's requirement when they passing or returning vector arguments. So Clang front-end will iterate the vector arguments and set `min-legal-vector-width` to the width of the maximum for both caller and callee.

It is assumed any middle end optimizations won't care of the attribute expect inlining and argument promotion.
- For inlining, we will propagate the attribute of inlined functions because the inlining functions become the newer caller.
- For argument promotion, we check the `min-legal-vector-width` of the caller and callee and refuse to promote when they don't match.

The problem comes from the optimizations' combination, as shown by https://godbolt.org/z/zo3hba8xW. The caller `foo` has two callees `bar` and `baz`. When doing argument promotion, both `foo` and `bar` has the same `min-legal-vector-width`. So the argument was promoted to vector. Then the inlining inlines `baz` to `foo` and updates `min-legal-vector-width`, which results in ABI mismatch between `foo` and `bar`.

This patch fixes the problem by expanding the concept of `min-legal-vector-width` to indicator of functions arguments. That says, any passes touch functions arguments have to set `min-legal-vector-width` to the value reflects the width of vector arguments. It makes sense to me because any arguments modifications are ABI related and should response for the ABI compatibility.

Differential Revision: https://reviews.llvm.org/D123284
  • Loading branch information
phoebewang committed May 2, 2022
1 parent e6295c6 commit 7c04454
Show file tree
Hide file tree
Showing 7 changed files with 134 additions and 51 deletions.
8 changes: 8 additions & 0 deletions llvm/docs/LangRef.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2188,6 +2188,14 @@ example:
unbounded. If the optional max value is omitted then max is set to the
value of min. If the attribute is not present, no assumptions are made
about the range of vscale.
``"min-legal-vector-width"="<size>"``
This attribute indicates the minimum legal vector width required by the
calling conversion. It is the maximum width of vector arguments and
returnings in the function and functions called by this function. Because
all the vectors are supposed to be legal type for compatibility.
Backends are free to ignore the attribute if they don't need to support
different maximum legal vector types or such information can be inferred by
other attributes.

Call Site Attributes
----------------------
Expand Down
3 changes: 3 additions & 0 deletions llvm/include/llvm/IR/Attributes.h
Original file line number Diff line number Diff line change
Expand Up @@ -1252,6 +1252,9 @@ void mergeAttributesForInlining(Function &Caller, const Function &Callee);
/// \param [in] ToMerge - The function to merge attributes from.
void mergeAttributesForOutlining(Function &Base, const Function &ToMerge);

/// Update min-legal-vector-width if it is in Attribute and less than Width.
void updateMinLegalVectorWidthAttr(Function &Fn, uint64_t Width);

} // end namespace AttributeFuncs

} // end namespace llvm
Expand Down
11 changes: 11 additions & 0 deletions llvm/lib/IR/Attributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2029,3 +2029,14 @@ void AttributeFuncs::mergeAttributesForOutlining(Function &Base,
// that aspect in the merged function.
mergeFnAttrs(Base, ToMerge);
}

void AttributeFuncs::updateMinLegalVectorWidthAttr(Function &Fn,
uint64_t Width) {
Attribute Attr = Fn.getFnAttribute("min-legal-vector-width");
if (Attr.isValid()) {
uint64_t OldWidth;
Attr.getValueAsString().getAsInteger(0, OldWidth);
if (Width > OldWidth)
Fn.addFnAttr("min-legal-vector-width", llvm::utostr(Width));
}
}
10 changes: 10 additions & 0 deletions llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,10 +218,17 @@ static Function *doPromotion(
LLVM_DEBUG(dbgs() << "ARG PROMOTION: Promoting to:" << *NF << "\n"
<< "From: " << *F);

uint64_t LargestVectorWidth = 0;
for (auto *I : Params)
if (auto *VT = dyn_cast<llvm::VectorType>(I))
LargestVectorWidth = std::max(
LargestVectorWidth, VT->getPrimitiveSizeInBits().getKnownMinSize());

// Recompute the parameter attributes list based on the new arguments for
// the function.
NF->setAttributes(AttributeList::get(F->getContext(), PAL.getFnAttrs(),
PAL.getRetAttrs(), ArgAttrVec));
AttributeFuncs::updateMinLegalVectorWidthAttr(*NF, LargestVectorWidth);
ArgAttrVec.clear();

F->getParent()->getFunctionList().insert(F->getIterator(), NF);
Expand Down Expand Up @@ -313,6 +320,9 @@ static Function *doPromotion(
Args.clear();
ArgAttrVec.clear();

AttributeFuncs::updateMinLegalVectorWidthAttr(*CB.getCaller(),
LargestVectorWidth);

// Update the callgraph to know that the callsite has been transformed.
if (ReplaceCallSite)
(*ReplaceCallSite)(CB, *NewCS);
Expand Down
10 changes: 10 additions & 0 deletions llvm/lib/Transforms/IPO/Attributor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2486,6 +2486,12 @@ ChangeStatus Attributor::rewriteFunctionSignatures(
}
}

uint64_t LargestVectorWidth = 0;
for (auto *I : NewArgumentTypes)
if (auto *VT = dyn_cast<llvm::VectorType>(I))
LargestVectorWidth = std::max(
LargestVectorWidth, VT->getPrimitiveSizeInBits().getKnownMinSize());

FunctionType *OldFnTy = OldFn->getFunctionType();
Type *RetTy = OldFnTy->getReturnType();

Expand Down Expand Up @@ -2515,6 +2521,7 @@ ChangeStatus Attributor::rewriteFunctionSignatures(
NewFn->setAttributes(AttributeList::get(
Ctx, OldFnAttributeList.getFnAttrs(), OldFnAttributeList.getRetAttrs(),
NewArgumentAttributes));
AttributeFuncs::updateMinLegalVectorWidthAttr(*NewFn, LargestVectorWidth);

// Since we have now created the new function, splice the body of the old
// function right into the new function, leaving the old rotting hulk of the
Expand Down Expand Up @@ -2592,6 +2599,9 @@ ChangeStatus Attributor::rewriteFunctionSignatures(
Ctx, OldCallAttributeList.getFnAttrs(),
OldCallAttributeList.getRetAttrs(), NewArgOperandAttributes));

AttributeFuncs::updateMinLegalVectorWidthAttr(*NewCB->getCaller(),
LargestVectorWidth);

CallSitePairs.push_back({OldCB, NewCB});
return true;
};
Expand Down
25 changes: 25 additions & 0 deletions llvm/test/Transforms/ArgumentPromotion/min-legal-vector-width.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
; RUN: opt < %s -passes=argpromotion -S | FileCheck %s

; CHECK-LABEL: define i32 @foo() #0 {
; CHECK-NEXT: %.val = load <32 x half>, <32 x half>* undef, align 4
; CHECK-NEXT: call void @bar(<32 x half> %.val)
; CHECK-NEXT: ret i32 0
; CHECK-NEXT: }

; CHECK-LABEL: define internal void @bar(<32 x half> %.0.val) #0 {
; CHECK-NEXT: ret void
; CHECK-NEXT: }

; CHECK: attributes #0 = { uwtable "min-legal-vector-width"="512" }

define i32 @foo() #0 {
call void @bar(<32 x half>* undef)
ret i32 0
}

define internal void @bar(<32 x half>*) #0 {
%2 = load <32 x half>, <32 x half>* %0, align 4
ret void
}

attributes #0 = { uwtable "min-legal-vector-width"="0" }
Loading

0 comments on commit 7c04454

Please sign in to comment.