Thank you very much for verifying this. I’ll make sure to include the bootstrap and regression-test results in future patch submissions.
Thanks a lot to both of you. I learned a lot from this patch. Have a great day. Best, Yongqiang. On Mon, 21 Sept 2026 at 04:22, Jeff Law <[email protected]> wrote: > > > > On 9/18/26 6:08 AM, Yongqiang Tian wrote: > > A DEBUG_INSN VAR_LOCATION can mention if_info->x without representing an > > ordinary use. Exclude debug instructions from the overlap scan so that > > debug-only payloads do not suppress min/max if-conversion. > > > > Assisted-by: OpenAI Codex 5.6 Sol, co-reviewed by Claude and Yongqiang Tian. > > > > PR rtl-optimization/127423 > > > > gcc/ChangeLog: > > > > * ifcvt.cc (noce_get_alt_condition): Ignore debug instructions when > > checking whether if_info->x is mentioned in the interval. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/riscv/pr127423.c: New test. > > > > Signed-off-by: Yongqiang Tian <[email protected]> > > --- > > Thanks for pointing this out. I checked the RTL immediately after `ce2`, > > rather than relying only on what var-tracking eventually produces. > > > > For the reported case, the transformed sequence first binds `xghost` to the > > old value of `if_info->x`, then defines `if_info->x` with `smin`, and only > > then > > binds `result` to the new value. The register is already defined at the > > first > > debug bind, and no ordinary instruction changes it before that program > > point. > > > > This ordering also follows from the interval involved here: the check scans > > `(cond_earliest, jump]`, while the merged definition is emitted at the > > former > > jump position. Ignoring the debug instruction in this membership check does > > not move the bind across the new definition. > > > > As a secondary check, var-tracking represents `xghost` using the entry value > > when that value is recoverable. In an additional variant where an opaque > > read-write asm operand prevents recovery of the old value, var-tracking > > makes > > `xghost` unavailable instead of associating it with the new minimum. Based > > on > > these checks, I do not think an explicit reset is needed in > > `noce_get_alt_condition` for this transformation. > > > > I also strengthened the regression test. `-fcompare-debug` alone can pass > > vacuously if `ce2` stops performing the conversion in both compilations. > > The > > test now compiles explicitly with `-g`, retains `-fcompare-debug`, and > > checks > > that the resulting assembly contains exactly one `min`. The strengthened > > test produces 14 expected passes with the patched compiler. The pristine > > compiler reports seven `-fcompare-debug failure (length)` failures, and a > > separate control with `ce2` disabled fails the assembly check. > > > > Thank you very much for your help. Any further suggestions would be greatly > > appreciated. > > > > Changes in v2: > > > > - Compile the regression test explicitly with `-g`. > > - Retain `-fcompare-debug` to check debug/no-debug convergence. > > - Require exactly one RISC-V `min` instruction so the test cannot pass > > without > > exercising the intended if-conversion. > So the patch changes a generic part of the compiler, so it needs to be > bootstrapped and regression tested on one of the major platforms > (typically x86_64 or aarch64). I went ahead and did that for you and > they passed without any issues. > > As I noted elsewhere in this thread, ignoring DEBUG_INSNs when doing a > search for "is this reg used in this context" is absolutely the right > thing to do, independent of any other concerns that may be raised about > the ifcvt.cc code. > > I've pushed this to the trunk. > > Thanks, > jeff > >
