[PATCH] D80618: Extend InvokeInst !prof branch_weights metadata to unwind branches

[wael yehia via Phabricator via llvm-commits]

w2yehia added a comment.

Hi @yrouban  
this commit breaks the following testcase. If you agree this is a regression, then can you please revert your commit until this is addressed.

  ; RUN: opt -simplifycfg -S %s
  ;
  target datalayout = "e-m:e-i64:64-n32:64"
  target triple = "powerpc64le-unknown-linux-gnu"
  
  %cM = type { i32 }
  %cE = type { i32 }
  
  @use = dso_local unnamed_addr alias void (%cM*), void (%cM*)* @foo
  
  define dso_local void @foo(%cM* %this) unnamed_addr align 2 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) !prof !0 {
  entry:
    %0 = load void (%cE, %cM*)*, void (%cE, %cM*)** undef, align 8
    invoke void %0(%cE undef, %cM* %this)
            to label %cond.end unwind label %lpad
  
  cond.end:                                         ; preds = %entry
    invoke void @bar()
            to label %for.cond unwind label %lpad22.loopexit.split-lp, !prof !1
  
  for.cond:                             ; preds = %cond.end
    unreachable
  
  lpad:                                             ; preds = %entry
    %1 = landingpad { i8*, i32 }
            catch i8* null
    unreachable
  
  lpad22.loopexit.split-lp:                         ; preds = %cond.end
    %lpad.loopexit.split-lp = landingpad { i8*, i32 }
            catch i8* null
    unreachable
  }
  
  declare i32 @__gxx_personality_v0(...)
  
  declare dso_local void @bar() local_unnamed_addr align 2
  
  !0 = !{!"function_entry_count", i64 9}
  !1 = !{!"branch_weights", i32 9, i32 0}

The verifier throws:

  Wrong number of operands
  !1 = !{!"branch_weights", i32 9, i32 0}

The issue is that SimplifyCFG tries to replace an invoke with a call, and copies over the MD as is.
One quick fix is

  --- a/llvm/lib/IR/Instruction.cpp
  +++ b/llvm/lib/IR/Instruction.cpp
  @@ -749,9 +749,14 @@ void Instruction::copyMetadata(const Instruction &SrcInst,
     // new one.
     SmallVector<std::pair<unsigned, MDNode *>, 4> TheMDs;
     SrcInst.getAllMetadataOtherThanDebugLoc(TheMDs);
  +  bool fixupBranchWeight = isa<CallInst>(this) && isa<InvokeInst>(SrcInst);
     for (const auto &MD : TheMDs) {
  -    if (WL.empty() || WLS.count(MD.first))
  -      setMetadata(MD.first, MD.second);
  +    if (!WL.empty() && !WLS.count(MD.first))
  +      continue;
  +    MDNode *MDN = MD.second;
  +    if (fixupBranchWeight && MDN->getNumOperands() == 3)
  +      MDN = MDNode::get(MDN->getContext(), {MDN->getOperand(0), MDN->getOperand(1)});
  +    setMetadata(MD.first, MDN);
     }
     if (WL.empty() || WLS.count(LLVMContext::MD_dbg))
       setDebugLoc(SrcInst.getDebugLoc());

But i'm not sure if it's the best one so I'll leave to you to decide.

Thank you for your attention.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80618/new/

https://reviews.llvm.org/D80618