On Wed, Sep 16, 2026 at 10:55 PM Yongqiang Tian <[email protected]> wrote: > > Thanks for pointing this out. I had not considered the debug-value > lifetime consequence when I sent the patch. > > The reason for the proposed change is that the DEBUG_INSN in the > reproducer mentions if_info->x before the jump. Treating that debug-only > reference as an ordinary use makes noce_get_alt_condition reject the > min/max conversion with debug information, while the same ordinary RTL > is converted without debug information. > > However, simply ignoring that reference means that the generated min > instruction can subsequently overwrite the same pseudo. If the debug > location were allowed to continue referring to that register after the > overwrite, it would indeed describe the wrong value. > > I traced the testcase through ce2 and vartrack using the RTL dumps. After > if-conversion, the relevant ordering is: > > (var_location xghost (reg ... a)) > (set (reg ... a) (smin ...)) > > Thus, the original debug bind is still valid before the new smin. For > the submitted testcase, vartrack then preserves the old value after the > smin by changing the location to an entry_value expression. > > I also tested a stronger variant in which the incoming value of a is > first changed by an inline asm, so that the value held by xghost cannot > be recovered from the function entry value. In that case, vartrack emits: > > (var_location xghost (nil)) > > after the smin, rather than extending the overwritten register location. > > So, for these two cases, the later var-tracking pass handles both > outcomes: it recovers the old value when possible and marks the location > unavailable when it is not. The debug bind before the smin remains valid > up to the point where the register is overwritten. > > For completeness, I also tested the exact submitted patch, commit > 16893fa6ec3e10d4bfe605e3bfc8afe85fde8aa2, against the unmodified > baseline at f311d59c4d45f5042029647c88966e2e48330adf. > > For the native testing, build, host, and target were all > x86_64-pc-linux-gnu. I configured both trees with: > > configure --disable-multilib > > and ran: > > make -j8 bootstrap > make -j8 -k check > > The bootstrap and complete testsuite run finished successfully. Comparing > the generated .sum files against the baseline showed no new FAIL, XPASS, > ERROR, or UNRESOLVED results. The seven pre-existing unexpected outcomes > were identical between the baseline and patched trees. > > I also built a C cross compiler with build and host > x86_64-pc-linux-gnu and target riscv64-unknown-linux-gnu. The > gcc.target/riscv/pr127423.c regression test passed in all seven testsuite > variations: -O0, -O1, -O2, -O3 -g, -Os, -Og -g, and -Oz. > > That said, I am not sufficiently familiar with the intended RTL debug > maintenance rules to know whether relying on vartrack here is the right > design, or whether ifcvt is expected to reset or propagate the affected > debug uses explicitly when committing the transformation. Would you > prefer the latter? I would be happy to revise the patch accordingly.
What matters is the RTL IL semantics after the ifcvt transform, not so much what the vartrack machinery in the end makes of it (that matters in practice, of course). So if after ifcvt the debug-insn refers to a register that is not set or set to the wrong value the debug-insn should be at least reset. I can't tell from the above whether the IL is correct or not and whether this is by design or accidential with the testcase. Richard.
