Skip to content

Commit 54001da

Browse files
dm-vodopyanovdm-vodopyanov
committed
Apply CR comments
1 parent d1f4ec2 commit 54001da

File tree

5 files changed

+95
-86
lines changed

5 files changed

+95
-86
lines changed

‎llvm/lib/SYCLLowerIR/SYCLConditionalCallOnDevice.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@ SYCLConditionalCallOnDevicePass::run(Module &M, ModuleAnalysisManager &) {
103103
&M);
104104

105105
NewFCaller->setCallingConv(FCaller->getCallingConv());
106-
NewFCaller->setAttributes(FCaller->getAttributes());
107106

108107
DenseMap<CallInst *, CallInst *> OldCallsToNewCalls;
109108

‎llvm/lib/SYCLLowerIR/SYCLPropagateAspectsUsage.cpp‎

Lines changed: 88 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -350,9 +350,11 @@ AspectsSetTy getAspectsUsedByInstruction(const Instruction &I,
350350

351351
using FunctionToAspectsMapTy = DenseMap<Function *, AspectsSetTy>;
352352
using ConditionsSetTy = SmallSet<int, 4>;
353-
using FunctionToConditionExpressionsMapTy = DenseMap<Function *, ConditionsSetTy>;
354-
using ConditionExpressionsAndConditionalAspectsTy = SmallVector<std::pair<ConditionsSetTy, AspectsSetTy>>;
355-
using FunctionToConditionExpressionsAndConditionalAspectsTy = DenseMap<Function *, ConditionExpressionsAndConditionalAspectsTy>;
353+
using FunctionToConditionExprsMapTy = DenseMap<Function *, ConditionsSetTy>;
354+
using ConditionExprsAndConditionalAspectsTy =
355+
SmallVector<std::pair<ConditionsSetTy, AspectsSetTy>>;
356+
using FuncToConditionExprsAndConditionalAspectsTy =
357+
DenseMap<Function *, ConditionExprsAndConditionalAspectsTy>;
356358
using FunctionSetTy = SmallPtrSet<Function *, 8>;
357359
using CallGraphTy = DenseMap<Function *, FunctionSetTy>;
358360

@@ -447,7 +449,7 @@ void createUsedAspectsMetadataForFunctions(
447449
}
448450

449451
void createConditionallyUsedAspectsMetadataForFunctions(
450-
FunctionToConditionExpressionsAndConditionalAspectsTy
452+
FuncToConditionExprsAndConditionalAspectsTy
451453
&FunctionToConditionExpressionsAndConditionalAspects,
452454
const AspectsSetTy &ExcludeAspectVals) {
453455
for (auto &[F, ConditionsAndAspectsVec] :
@@ -456,13 +458,12 @@ void createConditionallyUsedAspectsMetadataForFunctions(
456458

457459
// Create a set of unique conditions and aspects. First we add the ones from
458460
// the found aspects that have not been excluded.
459-
ConditionExpressionsAndConditionalAspectsTy UniqueConditionsAndAspects;
461+
ConditionExprsAndConditionalAspectsTy UniqueConditionsAndAspects;
460462
for (const auto &ConditionsAndAspectsPair : ConditionsAndAspectsVec) {
461463
AspectsSetTy FilteredAspects;
462464
for (const auto &A : ConditionsAndAspectsPair.second)
463-
if (!ExcludeAspectVals.contains(A)) {
465+
if (!ExcludeAspectVals.contains(A))
464466
FilteredAspects.insert(A);
465-
}
466467

467468
if (FilteredAspects.empty())
468469
continue;
@@ -498,6 +499,9 @@ void createConditionallyUsedAspectsMetadataForFunctions(
498499
AspectsSet.insert(cast<ConstantInt>(C)->getSExtValue());
499500
}
500501
}
502+
if (ConditionsSet.empty() || AspectsSet.empty())
503+
continue;
504+
501505
UniqueConditionsAndAspects.push_back(
502506
std::make_pair(ConditionsSet, AspectsSet));
503507
}
@@ -609,25 +613,25 @@ struct FunctionsClassifiedByTheirFunctionCall {
609613
FunctionSetTy CalledConditionally;
610614
};
611615

612-
void addAllCalledFunctionsRecursivelyToFunctionsClassifiedByTheirFunctionCallStruct(
613-
Function *F, const CallGraphTy &CG,
614-
FunctionsClassifiedByTheirFunctionCall &Funcs,
615-
SmallPtrSet<const Function *, 16> &Visited) {
616+
void classifyFuncsInCallGraph(Function *F, const CallGraphTy &CG,
617+
FunctionsClassifiedByTheirFunctionCall &Funcs,
618+
SmallPtrSet<const Function *, 16> &Visited) {
616619
const auto It = CG.find(F);
617620
if (It == CG.end())
618621
return;
622+
bool FCalledUnconditionally = Funcs.CalledUnconditionally.contains(F);
623+
bool FCalledConditionally = Funcs.CalledConditionally.contains(F);
619624
for (Function *Callee : It->second) {
620-
if (!Callee->hasFnAttribute("sycl-call-if-on-device-conditionally") &&
621-
Funcs.CalledUnconditionally.contains(F)) {
625+
bool CalleeIsCalledOnDeviceConditionally =
626+
Callee->hasFnAttribute("sycl-call-if-on-device-conditionally");
627+
if (!CalleeIsCalledOnDeviceConditionally && FCalledUnconditionally) {
622628
Funcs.CalledUnconditionally.insert(Callee);
623629
}
624-
if (Callee->hasFnAttribute("sycl-call-if-on-device-conditionally") ||
625-
Funcs.CalledConditionally.contains(F)) {
630+
if (CalleeIsCalledOnDeviceConditionally || FCalledConditionally) {
626631
Funcs.CalledConditionally.insert(Callee);
627632
}
628633
if (Visited.insert(Callee).second)
629-
addAllCalledFunctionsRecursivelyToFunctionsClassifiedByTheirFunctionCallStruct(
630-
Callee, CG, Funcs, Visited);
634+
classifyFuncsInCallGraph(Callee, CG, Funcs, Visited);
631635
}
632636
}
633637

@@ -685,74 +689,79 @@ void identifyPathsContainingConditionalCallers(
685689

686690
void propagateConditionExpressionsAndConditionalAspectsThroughCG(
687691
const PathsContainingConditionalCallersTy &Paths,
688-
FunctionToConditionExpressionsAndConditionalAspectsTy
689-
&ConditionsAndAspectsMap,
690-
FunctionToConditionExpressionsMapTy &ConditionsMap,
692+
FuncToConditionExprsAndConditionalAspectsTy &ConditionsAndAspectsMap,
693+
FunctionToConditionExprsMapTy &ConditionsMap,
691694
FunctionToAspectsMapTy &AspectsMap) {
692695
auto Contains =
693-
[](ConditionExpressionsAndConditionalAspectsTy ConditionsAndAspectsVec,
694-
std::pair<ConditionsSetTy, AspectsSetTy> Search) {
695-
for (const auto &ConditionAndAspectsPair : ConditionsAndAspectsVec)
696-
if ((ConditionAndAspectsPair.first == Search.first) &&
697-
(ConditionAndAspectsPair.second == Search.second))
698-
return true;
699-
return false;
696+
[](const ConditionExprsAndConditionalAspectsTy &ConditionsAndAspectsVec,
697+
const std::pair<ConditionsSetTy, AspectsSetTy> &Search) {
698+
return std::any_of(
699+
ConditionsAndAspectsVec.begin(), ConditionsAndAspectsVec.end(),
700+
[&Search](const auto ConditionAndAspectsPair) {
701+
return (ConditionAndAspectsPair.first == Search.first) &&
702+
(ConditionAndAspectsPair.second == Search.second);
703+
});
700704
};
701705

702706
// TODO: need to optimize finding duplicates and combining pairs with the same
703707
// Condition Expressions or Aspects
704708
for (const auto &Path : Paths)
705-
for (int I = Path.size() - 1; I >= 0; --I) {
706-
if (I != Path.size() - 1) {
709+
for (int E = Path.size() - 1, I = E; I >= 0; --I) {
710+
if (I != E) {
707711
const auto &CalleeAspects = AspectsMap[Path[I + 1]];
708712
AspectsMap[Path[I]].insert(CalleeAspects.begin(), CalleeAspects.end());
709713
if (ConditionsMap[Path[I]].empty())
710714
ConditionsMap[Path[I]] = ConditionsMap[Path[I + 1]];
711715
}
712716
auto NewPair =
713717
std::make_pair(ConditionsMap[Path[I]], AspectsMap[Path[I]]);
714-
if ((!NewPair.first.empty()) && (!NewPair.second.empty())) {
718+
if (!NewPair.first.empty() && !NewPair.second.empty()) {
715719
if (ConditionsAndAspectsMap[Path[I]].empty())
716720
ConditionsAndAspectsMap[Path[I]].push_back(NewPair);
717-
else
718-
if (!Contains(ConditionsAndAspectsMap[Path[I]], NewPair))
719-
ConditionsAndAspectsMap[Path[I]].push_back(NewPair);
720-
}
721-
if (I != Path.size() - 1)
722-
for (const auto &ConditionAndAspectsPair : ConditionsAndAspectsMap[Path[I + 1]])
723-
if (!Contains(ConditionsAndAspectsMap[Path[I]], ConditionAndAspectsPair))
724-
ConditionsAndAspectsMap[Path[I]].push_back(ConditionAndAspectsPair);
721+
else if (!Contains(ConditionsAndAspectsMap[Path[I]], NewPair))
722+
ConditionsAndAspectsMap[Path[I]].push_back(NewPair);
725723
}
724+
if (I != E)
725+
for (const auto &ConditionAndAspectsPair :
726+
ConditionsAndAspectsMap[Path[I + 1]])
727+
if (!Contains(ConditionsAndAspectsMap[Path[I]],
728+
ConditionAndAspectsPair))
729+
ConditionsAndAspectsMap[Path[I]].push_back(ConditionAndAspectsPair);
730+
}
726731
}
727732

728733
void propagateConditionExpressionsThroughCG(
729734
Function *F, const CallGraphTy &CG,
730-
FunctionToConditionExpressionsMapTy &ConditionsMap) {
731-
// TODO: re-write using Paths from identifyPathsContainingConditionalCallers func
735+
FunctionToConditionExprsMapTy &ConditionsMap) {
736+
// TODO: re-write using Paths from identifyPathsContainingConditionalCallers
737+
// func
732738
const auto It = CG.find(F);
733739
if (It == CG.end())
734740
return;
735741

736-
FunctionToConditionExpressionsMapTy LocalConditions;
742+
FunctionToConditionExprsMapTy LocalConditions;
737743
for (Instruction &I : instructions(F)) {
738-
if (const auto *CI = dyn_cast<CallInst>(&I)) {
739-
if (Function *CalledFunction = CI->getCalledFunction();
740-
!CI->isIndirectCall() && CalledFunction) {
741-
if (CalledFunction->hasFnAttribute(
742-
"sycl-call-if-on-device-conditionally")) {
743-
// Start the loop with the 2nd argument (counting from 0) as 0 arg by
744-
// design doc is Conditional Action, 1st arg is "this" pointer for the
745-
// application's callable object, and all Condition Expressions start
746-
// with 2nd argument
747-
for (unsigned J = 2; J < CI->arg_size(); ++J) {
748-
Value *Arg = CI->getArgOperand(J);
749-
if (auto *ConstInt = dyn_cast<ConstantInt>(Arg)) {
750-
// Get the integer value and add it to the set
751-
LocalConditions[CalledFunction].insert(
752-
static_cast<int>(ConstInt->getSExtValue()));
753-
}
754-
}
755-
}
744+
const auto *CI = dyn_cast<CallInst>(&I);
745+
if (!CI)
746+
continue;
747+
748+
Function *CalledFunction = CI->getCalledFunction();
749+
if (CI->isIndirectCall() || !CalledFunction)
750+
continue;
751+
752+
if (!CalledFunction->hasFnAttribute("sycl-call-if-on-device-conditionally"))
753+
continue;
754+
755+
// Start the loop with the 2nd argument (counting from 0) as 0 arg by
756+
// design doc is Conditional Action, 1st arg is "this" pointer for the
757+
// application's callable object, and all Condition Expressions start
758+
// with 2nd argument
759+
for (unsigned J = 2; J < CI->arg_size(); ++J) {
760+
Value *Arg = CI->getArgOperand(J);
761+
if (auto *ConstInt = dyn_cast<ConstantInt>(Arg)) {
762+
// Get the integer value and add it to the set
763+
LocalConditions[CalledFunction].insert(
764+
static_cast<int>(ConstInt->getSExtValue()));
756765
}
757766
}
758767
}
@@ -898,28 +907,28 @@ CallGraphTy getCallGraph(Module &M) {
898907
CallGraphTy CG;
899908
for (Function &F : M.functions()) {
900909
for (Instruction &I : instructions(F)) {
901-
if (const auto *CI = dyn_cast<CallInst>(&I)) {
902-
if (Function *CalledFunction = CI->getCalledFunction();
903-
!CI->isIndirectCall() && CalledFunction) {
904-
if (CalledFunction->hasFnAttribute(
905-
"sycl-call-if-on-device-conditionally")) {
906-
if (Function *CallableObj =
907-
dyn_cast<Function>(CI->getArgOperand(0))) {
908-
CG[&F].insert(CalledFunction);
909-
CG[CalledFunction].insert(CallableObj);
910-
}
911-
} else
910+
const auto *CI = dyn_cast<CallInst>(&I);
911+
if (!CI)
912+
continue;
913+
if (Function *CalledFunction = CI->getCalledFunction();
914+
!CI->isIndirectCall() && CalledFunction) {
915+
if (CalledFunction->hasFnAttribute(
916+
"sycl-call-if-on-device-conditionally")) {
917+
if (Function *CallableObj =
918+
dyn_cast<Function>(CI->getArgOperand(0))) {
912919
CG[&F].insert(CalledFunction);
913-
}
920+
CG[CalledFunction].insert(CallableObj);
921+
}
922+
} else
923+
CG[&F].insert(CalledFunction);
914924
}
915925
}
916926
}
917927
return CG;
918928
}
919929

920930
/// Returns a map of functions with corresponding used aspects.
921-
std::tuple<FunctionToAspectsMapTy,
922-
FunctionToConditionExpressionsAndConditionalAspectsTy,
931+
std::tuple<FunctionToAspectsMapTy, FuncToConditionExprsAndConditionalAspectsTy,
923932
FunctionToAspectsMapTy>
924933
buildFunctionsToAspectsMap(Module &M, TypeToAspectsMapTy &TypesWithAspects,
925934
const AspectValueToNameMapTy &AspectValues,
@@ -945,8 +954,8 @@ buildFunctionsToAspectsMap(Module &M, TypeToAspectsMapTy &TypesWithAspects,
945954
SmallPtrSet<const Function *, 16> Visited;
946955
for (Function *F : EntryPoints) {
947956
addFunctionToFunctionsClassifiedByTheirFunctionCallStruct(F);
948-
addAllCalledFunctionsRecursivelyToFunctionsClassifiedByTheirFunctionCallStruct(
949-
F, CG, FuncsClassifiedByTheirFunctionCall, Visited);
957+
classifyFuncsInCallGraph(F, CG, FuncsClassifiedByTheirFunctionCall,
958+
Visited);
950959
}
951960
for (Function &F : M.functions())
952961
if (!Visited.contains(&F))
@@ -967,16 +976,16 @@ buildFunctionsToAspectsMap(Module &M, TypeToAspectsMapTy &TypesWithAspects,
967976

968977
Visited.clear();
969978

970-
FunctionToConditionExpressionsMapTy FunctionToConditionExpressions;
979+
FunctionToConditionExprsMapTy FunctionToConditionExpressions;
971980
std::vector<std::vector<Function *>> Paths;
972981
for (Function *F : EntryPoints) {
973-
propagateConditionExpressionsThroughCG(F, CG, FunctionToConditionExpressions);
982+
propagateConditionExpressionsThroughCG(F, CG,
983+
FunctionToConditionExpressions);
974984
std::vector<Function *> CurrentPath;
975985
identifyPathsContainingConditionalCallers(F, CG, CurrentPath, Paths);
976986
}
977987

978-
FunctionToConditionExpressionsAndConditionalAspectsTy
979-
FunctionToCondExpsAndCondAspects;
988+
FuncToConditionExprsAndConditionalAspectsTy FunctionToCondExpsAndCondAspects;
980989
propagateConditionExpressionsAndConditionalAspectsThroughCG(
981990
Paths, FunctionToCondExpsAndCondAspects, FunctionToConditionExpressions,
982991
FunctionToConditionallyUsedAspects);

‎llvm/test/SYCLLowerIR/ConditionalCallOnDevice/conditional-call-on-device-1.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ entry:
2828
ret void
2929
}
3030

31-
; CHECK: declare spir_func void @call_if_on_device_conditionally_PREFIX_1(ptr[[VAL1:.*]], ptr[[VAL2:.*]], i32[[VAL3:.*]], i32[[VAL4:.*]])
31+
; CHECK: declare spir_func void @call_if_on_device_conditionally_PREFIX_1(ptr, ptr, i32, i32)
3232

3333
attributes #2 = { convergent mustprogress norecurse nounwind "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
3434
attributes #6 = { convergent inlinehint mustprogress norecurse nounwind "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }

‎llvm/test/SYCLLowerIR/ConditionalCallOnDevice/conditional-call-on-device-2.ll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ entry:
4242
ret void
4343
}
4444

45-
; CHECK: declare spir_func void @call_if_on_device_conditionally1_PREFIX_1(ptr[[VAL1:.*]], ptr[[VAL2:.*]], i32[[VAL3:.*]], i32[[VAL4:.*]])
46-
; CHECK: declare spir_func void @call_if_on_device_conditionally2_PREFIX_2(ptr[[VAL1:.*]], ptr[[VAL2:.*]], i32[[VAL3:.*]], i32[[VAL4:.*]])
45+
; CHECK: declare spir_func void @call_if_on_device_conditionally1_PREFIX_1(ptr, ptr, i32, i32)
46+
; CHECK: declare spir_func void @call_if_on_device_conditionally2_PREFIX_2(ptr, ptr, i32, i32)
4747

4848
attributes #2 = { convergent mustprogress norecurse nounwind "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
4949
attributes #6 = { convergent inlinehint mustprogress norecurse nounwind "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }

‎llvm/test/SYCLLowerIR/PropagateAspectsUsage/PropogateConditionalAspects/conditional-aspects-1.ll‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
; |
1010
; CF1
1111
; |
12-
; F3 (C)
12+
; F3 (C, fp64)
1313
;
1414

1515
%Optional.A = type { i32 }
@@ -41,7 +41,8 @@ define spir_func void @func2() {
4141

4242
; CHECK: define spir_func void @func3() !sycl_conditionally_used_aspects ![[#ID2:]]
4343
define spir_func void @func3() {
44-
%tmp = alloca %Optional.C
44+
%tmp1 = alloca %Optional.C
45+
%tmp2 = alloca double
4546
ret void
4647
}
4748

@@ -64,6 +65,6 @@ attributes #1 = { "sycl-call-if-on-device-conditionally"="true" }
6465
; CHECK: ![[#ID2]] = !{![[#ID3:]]}
6566
; CHECK: ![[#ID3]] = !{![[#ID4:]], ![[#ID5:]]}
6667
; CHECK: ![[#ID4]] = !{i32 0}
67-
; CHECK: ![[#ID5]] = !{i32 3}
68+
; CHECK: ![[#ID5]] = !{i32 3, i32 6}
6869
; CHECK: ![[#ID6]] = !{i32 1}
6970
; CHECK: ![[#ID7]] = !{i32 2}

0 commit comments

Comments
 (0)