WholeProgramDevirt: Implement importing for uniform ret val opt.

[pcc]

Summary

Depends on D29844

[tejohnson]

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

1078 ↗

(On Diff #88059)

The function name should be available for each CallSite in each VirtualCallSite on the CSInfo.CallSites. But would the call sites be from more than one function?

[pcc]

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

1078 ↗

(On Diff #88059)

These are not the names of the functions containing the call sites but rather the names of the virtual functions being called.

I think we'll probably want to somehow move these remarks into the exporting phase as well.

[tejohnson]

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

1078 ↗

(On Diff #88059)

Actually, looking at the code I don't understand why we wouldn't be emitting a remark during the exporting phase. DevirtModule::tryUniformRetValOpt will set WasDevirt to true for each Target. That in turn is invoked from tryVirtualConstProp in run() in the part executed on the Export & regular LTO paths, which should cause the function to be added to DevirtTargets for which remarks are emitted.

What's the difference between the remarks emitted for this case in run(), and the ones issued from within applyUniformRetValOpt?

[pcc]

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

1078 ↗

(On Diff #88059)

I believe that there are two separate kinds of remarks.

1. http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/WholeProgramDevirt.cpp#280
2. http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/WholeProgramDevirt.cpp#1221

The first is per call site and mentions the name of one possible target of the virtual call (arbitrarily chosen), and the other is per virtual function and mentions whether any calls to that function have been devirtualized. With this code we will emit the second type of remark during the exporting phase, and the first (with the arbitrary target name missing) during the importing phase.

(@krasin was the original author of the remark code.)

[tejohnson]

LGTM, with comment added on test as suggested below.

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

1078 ↗

(On Diff #88059)

Yes looks like it. We probably couldn't move the per call site remarks to the exporting side then, but would need more info here to get the name of a targets emitted. Looks like we also emit both remark kinds for applySingleImplDevirt, but there we already have the function name on the importing side.

llvm/test/Transforms/WholeProgramDevirt/import.ll

37 ↗

(On Diff #88059)

Comment on why we can't do the optimization here (I see UniformRetVal listed for typeid2 in the yaml file, but I'm probably misunderstanding how that is applied).

llvm/trunk/lib/Transforms/IPO/WholeProgramDevirt.cpp

if (Res.TheKind == WholeProgramDevirtResolution::SingleImpl) {
     auto *SingleImpl = M.getOrInsertFunction(
         Res.SingleImplName, Type::getVoidTy(M.getContext()), nullptr);
 
     // This is the import phase so we should not be exporting anything.
     bool IsExported = false;
     applySingleImplDevirt(SlotInfo, SingleImpl, IsExported);
     assert(!IsExported);
   }
+
+  for (auto &CSByConstantArg : SlotInfo.ConstCSInfo) {
+    auto I = Res.ResByArg.find(CSByConstantArg.first);
+    if (I == Res.ResByArg.end())
+      continue;
+    auto &ResByArg = I->second;
+    // FIXME: We should figure out what to do about the "function name" argument
+    // to the apply* functions, as the function names are unavailable during the
+    // importing phase. For now we just pass the empty string. This does not
+    // impact correctness because the function names are just used for remarks.
+    switch (ResByArg.TheKind) {
+    case WholeProgramDevirtResolution::ByArg::UniformRetVal:
+      applyUniformRetValOpt(CSByConstantArg.second, "", ResByArg.Info);
+      break;
+    default:
+      break;
+    }
+  }
 }
 
 void DevirtModule::removeRedundantTypeTests() {
   auto True = ConstantInt::getTrue(M.getContext());
   for (auto &&U : NumUnsafeUsesForTypeTest) {
     if (U.second == 0) {
       U.first->replaceAllUsesWith(True);
       U.first->eraseFromParent();

llvm/trunk/test/Transforms/WholeProgramDevirt/Inputs/import-uniform-ret-val.yaml

+---
+TypeIdMap:
+  typeid1:
+    WPDRes:
+      0:
+        Kind: Indir
+        ResByArg:
+          1:
+            Kind: UniformRetVal
+            Info: 42
+  typeid2:
+    WPDRes:
+      8:
+        Kind: Indir
+        ResByArg:
+          1:
+            Kind: UniformRetVal
+            Info: 42
+...

llvm/trunk/test/Transforms/WholeProgramDevirt/import.ll

 ; RUN: opt -S -wholeprogramdevirt -wholeprogramdevirt-summary-action=import -wholeprogramdevirt-read-summary=%S/Inputs/import-single-impl.yaml < %s | FileCheck --check-prefixes=CHECK,SINGLE-IMPL %s
+; RUN: opt -S -wholeprogramdevirt -wholeprogramdevirt-summary-action=import -wholeprogramdevirt-read-summary=%S/Inputs/import-uniform-ret-val.yaml < %s | FileCheck --check-prefixes=CHECK,UNIFORM-RET-VAL %s
 
 target datalayout = "e-p:64:64"
 target triple = "x86_64-unknown-linux-gnu"
 
+; Test cases where the argument values are known and we can apply virtual
+; constant propagation.
+
 ; CHECK: define i32 @call1
 define i32 @call1(i8* %obj) {
   %vtableptr = bitcast i8* %obj to [3 x i8*]**
   %vtable = load [3 x i8*]*, [3 x i8*]** %vtableptr
   %vtablei8 = bitcast [3 x i8*]* %vtable to i8*
   %p = call i1 @llvm.type.test(i8* %vtablei8, metadata !"typeid1")
   call void @llvm.assume(i1 %p)
   %fptrptr = getelementptr [3 x i8*], [3 x i8*]* %vtable, i32 0, i32 0
   %fptr = load i8*, i8** %fptrptr
   %fptr_casted = bitcast i8* %fptr to i32 (i8*, i32)*
   ; SINGLE-IMPL: call i32 bitcast (void ()* @singleimpl1 to i32 (i8*, i32)*)
   %result = call i32 %fptr_casted(i8* %obj, i32 1)
+  ; UNIFORM-RET-VAL: ret i32 42
   ret i32 %result
 }
 
+; Test cases where the argument values are unknown, so we cannot apply virtual
+; constant propagation.
+
 ; CHECK: define i1 @call2
 define i1 @call2(i8* %obj) {
   %vtableptr = bitcast i8* %obj to [1 x i8*]**
   %vtable = load [1 x i8*]*, [1 x i8*]** %vtableptr
   %vtablei8 = bitcast [1 x i8*]* %vtable to i8*
   %pair = call {i8*, i1} @llvm.type.checked.load(i8* %vtablei8, i32 8, metadata !"typeid2")
   %fptr = extractvalue {i8*, i1} %pair, 0
   %p = extractvalue {i8*, i1} %pair, 1
   ; SINGLE-IMPL: br i1 true,
   br i1 %p, label %cont, label %trap
 
 cont:
   %fptr_casted = bitcast i8* %fptr to i1 (i8*, i32)*
   ; SINGLE-IMPL: call i1 bitcast (void ()* @singleimpl2 to i1 (i8*, i32)*)
+  ; UNIFORM-RET-VAL: call i1 %
   %result = call i1 %fptr_casted(i8* %obj, i32 undef)
   ret i1 %result
 
 trap:
   call void @llvm.trap()
   unreachable
 }
 
 ; SINGLE-IMPL-DAG: declare void @singleimpl1()
 ; SINGLE-IMPL-DAG: declare void @singleimpl2()
 
 declare void @llvm.assume(i1)
 declare void @llvm.trap()
 declare {i8*, i1} @llvm.type.checked.load(i8*, i32, metadata)
 declare i1 @llvm.type.test(i8*, metadata)