[PATCH] D112870: [MergeFunctions] Extend FunctionComparator to account for metadata arguments in intrinsics, and only ignore debug info metadata

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

I'm not a fan of how dbg intrinsics are handled here. I think if we don't want to compare these, the principled way to do that would be to skip them entirely, something along these lines:

  diff --git a/llvm/lib/Transforms/Utils/FunctionComparator.cpp b/llvm/lib/Transforms/Utils/FunctionComparator.cpp
  index 326864803d7c..f75b75a57544 100644
  --- a/llvm/lib/Transforms/Utils/FunctionComparator.cpp
  +++ b/llvm/lib/Transforms/Utils/FunctionComparator.cpp
  @@ -791,8 +791,10 @@ int FunctionComparator::cmpValues(const Value *L, const Value *R) const {
   // Test whether two basic blocks have equivalent behaviour.
   int FunctionComparator::cmpBasicBlocks(const BasicBlock *BBL,
                                          const BasicBlock *BBR) const {
  -  BasicBlock::const_iterator InstL = BBL->begin(), InstLE = BBL->end();
  -  BasicBlock::const_iterator InstR = BBR->begin(), InstRE = BBR->end();
  +  auto WithoutDebugL = BBL->instructionsWithoutDebug();
  +  auto WithoutDebugR = BBR->instructionsWithoutDebug();
  +  auto InstL = WithoutDebugL.begin(), InstLE = WithoutDebugL.end();
  +  auto InstR = WithoutDebugR.begin(), InstRE = WithoutDebugR.end();
   
     do {
       bool needToCmpOperands = true;
  @@ -962,7 +964,7 @@ FunctionComparator::FunctionHash FunctionComparator::functionHash(Function &F) {
       // This random value acts as a block header, as otherwise the partition of
       // opcodes into BBs wouldn't affect the hash, only the order of the opcodes
       H.add(45798);
  -    for (auto &Inst : *BB) {
  +    for (auto &Inst : BB->instructionsWithoutDebug()) {
         H.add(Inst.getOpcode());
       }
       const Instruction *Term = BB->getTerminator();

And then always compare metadata operands on top of that.


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

https://reviews.llvm.org/D112870