[clang] 69183f8 - [NFC][Clang] Address reviews about overrideFunctionFeaturesWithTargetFeatures (#65938)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 20 04:37:17 PDT 2023
Author: Juan Manuel Martinez Caamaño
Date: 2023-09-20T13:37:13+02:00
New Revision: 69183f8eb97d3055b2a2617be85e8cb39d877acc
URL: https://github.com/llvm/llvm-project/commit/69183f8eb97d3055b2a2617be85e8cb39d877acc
DIFF: https://github.com/llvm/llvm-project/commit/69183f8eb97d3055b2a2617be85e8cb39d877acc.diff
LOG: [NFC][Clang] Address reviews about overrideFunctionFeaturesWithTargetFeatures (#65938)
Addressing remarks after merge of D159257
* Add comment
* Remove irrelevant CHECKs from test
* Simplify function
* Use llvm::sort before setting target-features as it is done in
CodeGenModeule
Added:
Modified:
clang/lib/CodeGen/CGCall.cpp
clang/test/CodeGen/link-builtin-bitcode.c
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index e15a4634b1d041b..3fe8d0a7cf49e16 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -2001,52 +2001,41 @@ static void getTrivialDefaultFunctionAttributes(
}
}
+/// Merges `target-features` from \TargetOpts and \F, and sets the result in
+/// \FuncAttr
+/// * features from \F are always kept
+/// * a feature from \TargetOpts is kept if itself and its opposite are absent
+/// from \F
static void
overrideFunctionFeaturesWithTargetFeatures(llvm::AttrBuilder &FuncAttr,
const llvm::Function &F,
const TargetOptions &TargetOpts) {
auto FFeatures = F.getFnAttribute("target-features");
- llvm::StringSet<> IncompatibleFeatureNames;
+ llvm::StringSet<> MergedNames;
SmallVector<StringRef> MergedFeatures;
MergedFeatures.reserve(TargetOpts.Features.size());
- if (FFeatures.isValid()) {
- const auto &TFeatures = TargetOpts.FeatureMap;
- for (StringRef Feature : llvm::split(FFeatures.getValueAsString(), ',')) {
+ auto AddUnmergedFeatures = [&](auto &&FeatureRange) {
+ for (StringRef Feature : FeatureRange) {
if (Feature.empty())
continue;
-
- bool EnabledForFunc = Feature.starts_with("+");
- assert(EnabledForFunc || Feature.starts_with("-"));
-
+ assert(Feature[0] == '+' || Feature[0] == '-');
StringRef Name = Feature.drop_front(1);
- auto TEntry = TFeatures.find(Name);
-
- // Preserves features that are incompatible (either set to something
- //
diff erent or missing) from the target features
- bool MissingFromTarget = TEntry == TFeatures.end();
- bool EnabledForTarget = !MissingFromTarget && TEntry->second;
- bool Incompatible = EnabledForTarget != EnabledForFunc;
- if (MissingFromTarget || Incompatible) {
+ bool Merged = !MergedNames.insert(Name).second;
+ if (!Merged)
MergedFeatures.push_back(Feature);
- if (Incompatible)
- IncompatibleFeatureNames.insert(Name);
- }
}
- }
+ };
- for (StringRef Feature : TargetOpts.Features) {
- if (Feature.empty())
- continue;
- StringRef Name = Feature.drop_front(1);
- if (IncompatibleFeatureNames.contains(Name))
- continue;
- MergedFeatures.push_back(Feature);
- }
+ if (FFeatures.isValid())
+ AddUnmergedFeatures(llvm::split(FFeatures.getValueAsString(), ','));
+ AddUnmergedFeatures(TargetOpts.Features);
- if (!MergedFeatures.empty())
+ if (!MergedFeatures.empty()) {
+ llvm::sort(MergedFeatures);
FuncAttr.addAttribute("target-features", llvm::join(MergedFeatures, ","));
+ }
}
void CodeGen::mergeDefaultFunctionDefinitionAttributes(
diff --git a/clang/test/CodeGen/link-builtin-bitcode.c b/clang/test/CodeGen/link-builtin-bitcode.c
index fe60a9746f1c85f..470180efa4247aa 100644
--- a/clang/test/CodeGen/link-builtin-bitcode.c
+++ b/clang/test/CodeGen/link-builtin-bitcode.c
@@ -43,7 +43,7 @@ int bar() { return no_attr() + attr_in_target() + attr_not_in_target() + attr_in
// CHECK-LABEL: @attr_incompatible
// CHECK-SAME: () #[[ATTR_INCOMPATIBLE:[0-9]+]] {
-// CHECK: attributes #[[ATTR_BAR]] = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="gfx90a" "target-features"="+16-bit-insts,+atomic-buffer-global-pk-add-f16-insts,+atomic-fadd-rtn-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot3-insts,+dot4-insts,+dot5-insts,+dot6-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+gfx90a-insts,+mai-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64" }
-// CHECK: attributes #[[ATTR_COMPATIBLE]] = { convergent noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="gfx90a" "target-features"="+16-bit-insts,+atomic-buffer-global-pk-add-f16-insts,+atomic-fadd-rtn-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot3-insts,+dot4-insts,+dot5-insts,+dot6-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+gfx90a-insts,+gws,+image-insts,+mai-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64" }
-// CHECK: attributes #[[ATTR_EXTEND]] = { convergent noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="gfx90a" "target-features"="+extended-image-insts,+16-bit-insts,+atomic-buffer-global-pk-add-f16-insts,+atomic-fadd-rtn-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot3-insts,+dot4-insts,+dot5-insts,+dot6-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+gfx90a-insts,+gws,+image-insts,+mai-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64" }
-// CHECK: attributes #[[ATTR_INCOMPATIBLE]] = { convergent noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="gfx90a" "target-features"="-gfx9-insts,+16-bit-insts,+atomic-buffer-global-pk-add-f16-insts,+atomic-fadd-rtn-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot3-insts,+dot4-insts,+dot5-insts,+dot6-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx90a-insts,+gws,+image-insts,+mai-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64" }
+// CHECK: attributes #[[ATTR_BAR]] = { {{.*}} "target-cpu"="gfx90a" "target-features"="+16-bit-insts,+atomic-buffer-global-pk-add-f16-insts,+atomic-fadd-rtn-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot3-insts,+dot4-insts,+dot5-insts,+dot6-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+gfx90a-insts,+mai-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64" }
+// CHECK: attributes #[[ATTR_COMPATIBLE]] = { {{.*}} "target-cpu"="gfx90a" "target-features"="+16-bit-insts,+atomic-buffer-global-pk-add-f16-insts,+atomic-fadd-rtn-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot3-insts,+dot4-insts,+dot5-insts,+dot6-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+gfx90a-insts,+gws,+image-insts,+mai-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64" }
+// CHECK: attributes #[[ATTR_EXTEND]] = { {{.*}} "target-cpu"="gfx90a" "target-features"="+16-bit-insts,+atomic-buffer-global-pk-add-f16-insts,+atomic-fadd-rtn-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot3-insts,+dot4-insts,+dot5-insts,+dot6-insts,+dot7-insts,+dpp,+extended-image-insts,+gfx8-insts,+gfx9-insts,+gfx90a-insts,+gws,+image-insts,+mai-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64" }
+// CHECK: attributes #[[ATTR_INCOMPATIBLE]] = { {{.*}} "target-cpu"="gfx90a" "target-features"="+16-bit-insts,+atomic-buffer-global-pk-add-f16-insts,+atomic-fadd-rtn-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot3-insts,+dot4-insts,+dot5-insts,+dot6-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx90a-insts,+gws,+image-insts,+mai-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64,-gfx9-insts" }
More information about the cfe-commits
mailing list