[Transforms][IPO] Add remarks for ArgumentPromotion and DeadArgumentE… #105740

yonghong-song · 2024-08-22T21:49:54Z

…limination

ArgumentPromotion and DeadArgumentElimination passes may change function signature. This makes bpf tracing difficult since users either not aware of signature change or need to poke into IR or assembly to understand the function signature change.

This patch enabled to emit some remarks so if recompiling with -foptimization-record-file=, users can check remarks to see what kind of signature changes for a particular function. The following are some examples for implemented remarks:

  Pass:            deadargelim
  Name:            ReturnValueRemoved
  DebugLoc:        { File: 'bpf-next/net/mptcp/protocol.c', Line: 572, Column: 0 }
  Function:        mptcp_check_data_fin
  Args:
    - String:          'removing return value '
    - String:          '0'

  Pass:            deadargelim
  Name:            ArgumentRemoved
  DebugLoc:        { File: 'bpf-next/kernel/bpf/syscall.c', Line: 1670, Column: 0 }
  Function:        map_delete_elem
  Args:
      - String:          'eliminating argument '
      - ArgName:         uattr.coerce0
      - String:          '('
      - ArgIndex:        '1'
      - String:          ')'

  Pass:            argpromotion
  Name:            ArgumentPromoted
  DebugLoc:        { File: 'bpf-next/net/mptcp/protocol.h', Line: 570, Column: 0 }
  Function:        mptcp_subflow_ctx
  Args:
    - String:          'promoting argument '
    - ArgName:         sk
    - String:          '('
    - ArgIndex:        '0'
    - String:          ')'
    - String:          ' to pass by value'

[1] #104678

llvmbot · 2024-08-22T21:50:26Z

@llvm/pr-subscribers-llvm-transforms

Author: None (yonghong-song)

Changes

…limination

ArgumentPromotion and DeadArgumentElimination passes may change function signature. This makes bpf tracing difficult since users either not aware of signature change or need to poke into IR or assembly to understand the function signature change.

This patch enabled to emit some remarks so if recompiling with -foptimization-record-file=<file>, users can check remarks to see what kind of signature changes for a particular function. The following are some examples for implemented remarks:

  Pass:            deadargelim
  Name:            ReturnValueRemoved
  DebugLoc:        { File: 'bpf-next/net/mptcp/protocol.c', Line: 572, Column: 0 }
  Function:        mptcp_check_data_fin
  Args:
    - Function:        mptcp_check_data_fin
      DebugLoc:        { File: 'bpf-next/net/mptcp/protocol.c', Line: 572,
                         Column: 0 }
    - String:          ' removing return value '
    - String:          '0'

  Pass:            deadargelim
  Name:            ArgumentRemoved
  DebugLoc:        { File: 'bpf-next/kernel/bpf/syscall.c', Line: 1670, Column: 0 }
  Function:        map_delete_elem
  Args:
    - Function:        map_delete_elem
      DebugLoc:        { File: 'bpf-next/kernel/bpf/syscall.c', Line: 1670,
                         Column: 0 }
    - String:          ' removing argument '
    - String:          '1'
    - String:          ' ('
    - String:          uattr.coerce0
    - String:          ')'

  Pass:            argpromotion
  Name:            ArgumentPromoted
  DebugLoc:        { File: 'bpf-next/net/mptcp/protocol.h', Line: 570, Column: 0 }
  Function:        mptcp_subflow_ctx
  Args:
    - Function:        mptcp_subflow_ctx
      DebugLoc:        { File: 'bpf-next/net/mptcp/protocol.h', Line: 570,
                         Column: 0 }
    - String:          ' promoting argument '
    - String:          '0'
    - String:          ' ('
    - String:          sk
    - String:          ')'

[1] #104678

Full diff: https://github.com/llvm/llvm-project/pull/105740.diff

2 Files Affected:

(modified) llvm/lib/Transforms/IPO/ArgumentPromotion.cpp (+14)
(modified) llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp (+14)

diff --git a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
index 452fff7898d0ea..9369d2be74bb7d 100644
--- a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
+++ b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
@@ -42,6 +42,7 @@
 #include "llvm/Analysis/CallGraph.h"
 #include "llvm/Analysis/Loads.h"
 #include "llvm/Analysis/MemoryLocation.h"
+#include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/Argument.h"
@@ -126,6 +127,7 @@ doPromotion(Function *F, FunctionAnalysisManager &FAM,
   // arguments.
   SmallVector<unsigned> NewArgIndices;
   AttributeList PAL = F->getAttributes();
+  OptimizationRemarkEmitter ORE(F);
 
   // First, determine the new argument list
   unsigned ArgNo = 0, NewArgNo = 0;
@@ -139,6 +141,12 @@ doPromotion(Function *F, FunctionAnalysisManager &FAM,
     } else if (I->use_empty()) {
       // Dead argument (which are always marked as promotable)
       ++NumArgumentsDead;
+      ORE.emit([&]() {
+        return OptimizationRemark(DEBUG_TYPE, "ArgumentRemoved", F)
+               << ore::NV("Function", F) << " removing argument "
+               << std::to_string(ArgNo) << " (" << I->getName() << ")";
+      });
+
       NewArgIndices.push_back((unsigned)-1);
     } else {
       const auto &ArgParts = ArgsToPromote.find(&*I)->second;
@@ -147,6 +155,12 @@ doPromotion(Function *F, FunctionAnalysisManager &FAM,
         ArgAttrVec.push_back(AttributeSet());
       }
       ++NumArgumentsPromoted;
+      ORE.emit([&]() {
+        return OptimizationRemark(DEBUG_TYPE, "ArgumentPromoted", F)
+               << ore::NV("Function", F) << " promoting argument "
+               << std::to_string(ArgNo) << " (" << I->getName() << ")";
+      });
+
       NewArgIndices.push_back((unsigned)-1);
       NewArgNo += ArgParts.size();
     }
diff --git a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp b/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
index f5a7ab26a49e96..bb71993995cf18 100644
--- a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
+++ b/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Transforms/IPO/DeadArgumentElimination.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/IR/Argument.h"
 #include "llvm/IR/AttributeMask.h"
 #include "llvm/IR/Attributes.h"
@@ -748,6 +749,7 @@ bool DeadArgumentEliminationPass::removeDeadStuffFromFunction(Function *F) {
   // Set up to build a new list of parameter attributes.
   SmallVector<AttributeSet, 8> ArgAttrVec;
   const AttributeList &PAL = F->getAttributes();
+  OptimizationRemarkEmitter ORE(F);
 
   // Remember which arguments are still alive.
   SmallVector<bool, 10> ArgAlive(FTy->getNumParams(), false);
@@ -765,6 +767,12 @@ bool DeadArgumentEliminationPass::removeDeadStuffFromFunction(Function *F) {
       HasLiveReturnedArg |= PAL.hasParamAttr(ArgI, Attribute::Returned);
     } else {
       ++NumArgumentsEliminated;
+
+      ORE.emit([&]() {
+        return OptimizationRemark(DEBUG_TYPE, "ArgumentRemoved", F)
+               << ore::NV("Function", F) << " removing argument "
+               << std::to_string(ArgI) << " (" << I->getName() << ")";
+      });
       LLVM_DEBUG(dbgs() << "DeadArgumentEliminationPass - Removing argument "
                         << ArgI << " (" << I->getName() << ") from "
                         << F->getName() << "\n");
@@ -810,6 +818,12 @@ bool DeadArgumentEliminationPass::removeDeadStuffFromFunction(Function *F) {
         NewRetIdxs[Ri] = RetTypes.size() - 1;
       } else {
         ++NumRetValsEliminated;
+
+        ORE.emit([&]() {
+          return OptimizationRemark(DEBUG_TYPE, "ReturnValueRemoved", F)
+                 << ore::NV("Function", F) << " removing return value "
+                 << std::to_string(Ri);
+        });
         LLVM_DEBUG(
             dbgs() << "DeadArgumentEliminationPass - Removing return value "
                    << Ri << " from " << F->getName() << "\n");

yonghong-song · 2024-08-22T21:50:44Z

cc @4ast @eddyz87 @anakryiko

4ast · 2024-08-22T22:14:48Z

That should be pretty useful. Is there a tool that can parse it programmatically or, so far, it's for human consumption only?

yonghong-song · 2024-08-22T22:23:13Z

That should be pretty useful. Is there a tool that can parse it programmatically or, so far, it's for human consumption only?

Alexei, I know there is a meta internal tool to parse remarks to construct an inlining-decision tree. Maybe upstream have some tools to parse those remarks as well. I am not sure.

eddyz87 · 2024-08-23T00:19:53Z

llvm/lib/Transforms/IPO/ArgumentPromotion.cpp

@@ -139,6 +141,12 @@ doPromotion(Function *F, FunctionAnalysisManager &FAM,
    } else if (I->use_empty()) {
      // Dead argument (which are always marked as promotable)
      ++NumArgumentsDead;
+      ORE.emit([&]() {
+        return OptimizationRemark(DEBUG_TYPE, "ArgumentRemoved", F)
+               << ore::NV("Function", F) << " removing argument "


Nitpick: function name is printed twice, first for OptimizationRemark, next for NV, is it really necessary?

Good suggestion. The second function name is not needed. Will fix.

david-xl

Also add a test case?

yonghong-song · 2024-08-27T18:50:46Z

Also add a test case?

Looks like you are okay with the format? I will add a test case in the next revision.

david-xl · 2024-08-27T19:19:14Z

Also add a test case?

Looks like you are okay with the format? I will add a test case in the next revision.

Adding a case will help get a better feel of the format too.

yonghong-song · 2024-08-29T15:17:03Z

Also add a test case?

Looks like you are okay with the format? I will add a test case in the next revision.

Adding a case will help get a better feel of the format too.

@david-xl I just added two remark tests corresponding to optimization passes. Please take a look.

david-xl

bike shedding on the format. Is it better to show the parameter name first in the message?

foo: eliminating argument 'x' (index 0)
foo: promoting argument 'p'(index 1) to pass by value

…limination ArgumentPromotion and DeadArgumentElimination passes may change function signature. This makes bpf tracing difficult since users either not aware of signature change or need to poke into IR or assembly to understand the function signature change. This patch enabled to emit some remarks so if recompiling with -foptimization-record-file=<file>, users can check remarks to see what kind of signature changes for a particular function. The following are some examples for implemented remarks: Pass: deadargelim Name: ReturnValueRemoved DebugLoc: { File: 'bpf-next/net/mptcp/protocol.c', Line: 572, Column: 0 } Function: mptcp_check_data_fin Args: - String: 'removing return value ' - String: '0' Pass: deadargelim Name: ArgumentRemoved DebugLoc: { File: 'bpf-next/kernel/bpf/syscall.c', Line: 1670, Column: 0 } Function: map_delete_elem Args: - String: 'eliminating argument ' - ArgName: uattr.coerce0 - String: '(' - ArgIndex: '1' - String: ')' Pass: argpromotion Name: ArgumentPromoted DebugLoc: { File: 'bpf-next/net/mptcp/protocol.h', Line: 570, Column: 0 } Function: mptcp_subflow_ctx Args: - String: 'promoting argument ' - ArgName: sk - String: '(' - ArgIndex: '0' - String: ')' - String: ' to pass by value' [1] llvm#104678

yonghong-song · 2024-08-30T05:05:11Z

@david-xl I made some changes to the remark format based on your suggestion. Could you take a look?

david-xl

LGTM (you may want to wait a few days in case there are other comments)

nikic

Looks reasonable to me.

yonghong-song requested review from nikic and david-xl August 22, 2024 21:49

llvmbot added the llvm:transforms label Aug 22, 2024

eddyz87 reviewed Aug 23, 2024

View reviewed changes

yonghong-song force-pushed the add-remarks branch from ad5eef6 to 0ed11b0 Compare August 23, 2024 17:46

david-xl reviewed Aug 27, 2024

View reviewed changes

yonghong-song force-pushed the add-remarks branch from 0ed11b0 to 4ca7ead Compare August 29, 2024 15:16

david-xl reviewed Aug 29, 2024

View reviewed changes

yonghong-song force-pushed the add-remarks branch from 4ca7ead to b7f5d61 Compare August 30, 2024 05:02

david-xl approved these changes Aug 30, 2024

View reviewed changes

nikic approved these changes Aug 30, 2024

View reviewed changes

yonghong-song merged commit 470f55f into llvm:main Aug 31, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Transforms][IPO] Add remarks for ArgumentPromotion and DeadArgumentE… #105740

[Transforms][IPO] Add remarks for ArgumentPromotion and DeadArgumentE… #105740

Uh oh!

yonghong-song commented Aug 22, 2024 •

edited

Loading

Uh oh!

llvmbot commented Aug 22, 2024

Uh oh!

yonghong-song commented Aug 22, 2024

Uh oh!

4ast commented Aug 22, 2024

Uh oh!

yonghong-song commented Aug 22, 2024

Uh oh!

eddyz87 Aug 23, 2024

Uh oh!

yonghong-song Aug 23, 2024

Uh oh!

david-xl left a comment

Uh oh!

yonghong-song commented Aug 27, 2024

Uh oh!

david-xl commented Aug 27, 2024

Uh oh!

yonghong-song commented Aug 29, 2024

Uh oh!

david-xl left a comment

Uh oh!

yonghong-song commented Aug 30, 2024

Uh oh!

david-xl left a comment

Uh oh!

nikic left a comment

Uh oh!

Uh oh!

Uh oh!

[Transforms][IPO] Add remarks for ArgumentPromotion and DeadArgumentE… #105740

[Transforms][IPO] Add remarks for ArgumentPromotion and DeadArgumentE… #105740

Uh oh!

Conversation

yonghong-song commented Aug 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Aug 22, 2024

Uh oh!

yonghong-song commented Aug 22, 2024

Uh oh!

4ast commented Aug 22, 2024

Uh oh!

yonghong-song commented Aug 22, 2024

Uh oh!

eddyz87 Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

yonghong-song Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

david-xl left a comment

Choose a reason for hiding this comment

Uh oh!

yonghong-song commented Aug 27, 2024

Uh oh!

david-xl commented Aug 27, 2024

Uh oh!

yonghong-song commented Aug 29, 2024

Uh oh!

david-xl left a comment

Choose a reason for hiding this comment

Uh oh!

yonghong-song commented Aug 30, 2024

Uh oh!

david-xl left a comment

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

yonghong-song commented Aug 22, 2024 •

edited

Loading