[llvm] Reapply [Metadata] Preserve MD_prof when merging instructions when one is missing. (PR #135418)
Snehasish Kumar via llvm-commits
llvm-commits at lists.llvm.org
Sat Apr 12 16:14:33 PDT 2025
snehasish wrote:
> > Also add a legality check when merging prof metadata based on instruction type. Without this check GVN PRE optimizations result in prof metadata on phi nodes which break the module verifier.
>
> Can you please explain in more detail what happens? This sounds suspicious.
Sure I can walk through *what* happens, though I am not familiar with GVN so I don't know *why* it happens. This is the reduced IR --
```
define fastcc ptr @foo(i8 %__pred.1.val) {
entry:
switch i64 0, label %sw.bb [
i64 1, label %sw.bb26
i64 0, label %sw.bb21
]
sw.bb: ; preds = %entry
%l78 = trunc i8 %__pred.1.val to i1
%c79 = select i1 %l78, i64 0, i64 4
br label %sw.bb21
sw.bb21: ; preds = %sw.bb, %entry
%l83 = trunc i8 %__pred.1.val to i1
%c84 = select i1 %l83, i64 0, i64 4
br label %sw.bb26
sw.bb26: ; preds = %sw.bb21, %entry
%l88 = trunc i8 %__pred.1.val to i1
%c89 = select i1 %l88, i64 0, i64 4, !prof !0
ret ptr null
}
```
The first step is a change to the cfg to split the critical edges. After this we get --
```
define fastcc ptr @foo(i8 %__pred.1.val) {
entry:
switch i64 0, label %sw.bb [
i64 1, label %entry.sw.bb26_crit_edge
i64 0, label %entry.sw.bb21_crit_edge
]
entry.sw.bb21_crit_edge: ; preds = %entry
br label %sw.bb21
entry.sw.bb26_crit_edge: ; preds = %entry
br label %sw.bb26
sw.bb: ; preds = %entry
%l78 = trunc i8 %__pred.1.val to i1
%c79 = select i1 %l78, i64 0, i64 4
br label %sw.bb21
sw.bb21: ; preds = %entry.sw.bb21_crit_edge, %sw.bb
%l83 = trunc i8 %__pred.1.val to i1
%c84 = select i1 %l83, i64 0, i64 4
br label %sw.bb26
sw.bb26: ; preds = %entry.sw.bb26_crit_edge, %sw.bb21
%l88 = trunc i8 %__pred.1.val to i1
%c89 = select i1 %l88, i64 0, i64 4, !prof !0
ret ptr null
}
```
Then GVN moves `%l83` and `%c84` to `entry.sw.bb21_crit_edge:` and replaces them with phi nodes in `sw.bb21`. This transformation would *not* have triggered the behaviour if `%c79` had branch prof data. The IR after this step is:
```
define fastcc ptr @foo(i8 %__pred.1.val) {
entry:
switch i64 0, label %sw.bb [
i64 1, label %entry.sw.bb26_crit_edge
i64 0, label %entry.sw.bb21_crit_edge
]
entry.sw.bb21_crit_edge: ; preds = %entry
%.pre = trunc i8 %__pred.1.val to i1
%.pre1 = select i1 %.pre, i64 0, i64 4
br label %sw.bb21
entry.sw.bb26_crit_edge: ; preds = %entry
br label %sw.bb26
sw.bb: ; preds = %entry
%l78 = trunc i8 %__pred.1.val to i1
%c79 = select i1 %l78, i64 0, i64 4
br label %sw.bb21
sw.bb21: ; preds = %entry.sw.bb21_crit_edge, %sw.bb
%c84.pre-phi = phi i64 [ %.pre1, %entry.sw.bb21_crit_edge ], [ %c79, %sw.bb ]
%l83.pre-phi = phi i1 [ %.pre, %entry.sw.bb21_crit_edge ], [ %l78, %sw.bb ]
br label %sw.bb26
sw.bb26: ; preds = %entry.sw.bb26_crit_edge, %sw.bb21
%l88 = trunc i8 %__pred.1.val to i1
%c89 = select i1 %l88, i64 0, i64 4, !prof !0
ret ptr null
}
```
In the next step we, perform a similar transformation for the trunc and select (which has prof data) in `sw.bb26`. However this time we find the value in the PredMap and call `patchReplacementInstruction`. This in turn calls `combineMetadataForCSE` which ends up applying prof metadata onto a phi instruction. The IR after this step is shown below.
```
define fastcc ptr @foo(i8 %__pred.1.val) {
entry:
switch i64 0, label %sw.bb [
i64 1, label %entry.sw.bb26_crit_edge
i64 0, label %entry.sw.bb21_crit_edge
]
entry.sw.bb21_crit_edge: ; preds = %entry
%.pre = trunc i8 %__pred.1.val to i1
%.pre1 = select i1 %.pre, i64 0, i64 4
br label %sw.bb21
entry.sw.bb26_crit_edge: ; preds = %entry
%.pre2 = trunc i8 %__pred.1.val to i1
%.pre3 = select i1 %.pre2, i64 0, i64 4, !prof !0
br label %sw.bb26
sw.bb: ; preds = %entry
%l78 = trunc i8 %__pred.1.val to i1
%c79 = select i1 %l78, i64 0, i64 4
br label %sw.bb21
sw.bb21: ; preds = %entry.sw.bb21_crit_edge, %sw.bb
%c84.pre-phi = phi i64 [ %.pre1, %entry.sw.bb21_crit_edge ], [ %c79, %sw.bb ], !prof !0
%l83.pre-phi = phi i1 [ %.pre, %entry.sw.bb21_crit_edge ], [ %l78, %sw.bb ]
br label %sw.bb26
sw.bb26: ; preds = %entry.sw.bb26_crit_edge, %sw.bb21
%c89.pre-phi = phi i64 [ %.pre3, %entry.sw.bb26_crit_edge ], [ %c84.pre-phi, %sw.bb21 ]
%l88.pre-phi = phi i1 [ %.pre2, %entry.sw.bb26_crit_edge ], [ %l83.pre-phi, %sw.bb21 ]
ret ptr null
}
```
Regardless of whether this behaviour is intended in GVN, I think it's a good idea to check the legality of prof metadata before we attempt to merge.
https://github.com/llvm/llvm-project/pull/135418
More information about the llvm-commits
mailing list