llvm / llvm/llvm-project

[MachineLICM] Instructions that implicitly define flag registers can be hoisted into live ranges of those registers

Open
#219,183 6 comments 0 reactions 0 assignees View on GitHub
llvm:codegen miscompilation
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

This was discovered with csmith-based testing. It seems that, when hoisting an instruction with implicit physreg definitions out of a loop, the Early MachineLICM pass does not consider whether said flag operand may be live at the insertion point. It just looks at whether the operand might be live going into the loop header (via `MachineLoop::isLoopInvariant`). In the specific example in the attached repro, the instruction
```llvm-mir
%5:grx32bit = XIFMux %2:grx32bit(tied-def 0), 4294967295, implicit-def dead $cc
```
was moved
```llvm-mir
CGHSI killed %3:addr64bit, 0, 0, implicit-def $cc :: (dereferenceable load (s64) from @func_7_p_9, !tbaa !8)

BRC 14, 6, %bb.1, implicit $cc
```
As far as i can tell, the pass always moves the hoisted instruction to before the first terminator in the preheader, without considering the liveness of, e.g. `$cc`.

I have attached a repro script both for the original C code input, as well as a focused MIR repro for the pass itself.
[repro.tar.gz](https://github.com/user-attachments/files/31511601/repro.tar.gz)

I may be misinterpreting something because this seems like it has been like this for a while (i tested as far back as LLVM 15 with the MIR test), and would be an issue for more than just the SystemZ backend (i had an LLM generate a corresponding X86 MIR testcase and it did seem to trigger the same problematic behavior).

So my main question is - am i reading this right? And if so - what is the appropriate fix? Check for liveness at the insertion point and reject? Or have the Pass search for a safe insertion point looking at physreg liveness?

Contributor guide

Open the contributing guide

Research direction

Start with the focused MIR repro in repro.tar.gz and inspect Early MachineLICM's handling of MachineLoop::isLoopInvariant and preheader insertion. Trace the implicit $cc definition and its liveness at the insertion point, then determine whether the pass should reject the hoist or find a safe insertion point. Done means the behavior is validated and the chosen fix is covered by the MIR repro.

Written by the indexing model from the issue text.

Assessment

Domain
compilers
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.