On Tue, Jul 28, 2026 at 12:36 PM Roy, Reshma <[email protected]> wrote: > > AMD General > > > -----Original Message----- > > From: Uros Bizjak <[email protected]> > > Sent: Tuesday, July 28, 2026 11:56 AM > > To: Roy, Reshma <[email protected]> > > Cc: [email protected]; [email protected]; Kumar, Venkataramanan > > <[email protected]>; Aloor, Raghesh > > <[email protected]> > > Subject: Re: [PATCH] x86_cse: Check if DRAP is live with DF_LR_IN. > > > > [You don't often get email from [email protected]. Learn why this is > > important at > > https://aka.ms/LearnAboutSenderIdentification ] > > > > Caution: This message originated from an External Source. Use proper caution > > when opening attachments, clicking links, or responding. > > > > > > On Fri, Jul 24, 2026 at 6:47 PM <[email protected]> wrote: > > > > > > From: Reshma Roy <[email protected]> > > > > > > The DRAP register has no reaching definition on function entry, so it > > > never shows up in DF_LIVE_IN. When collecting the live caller-saved > > > registers, additionally set DRAP's bit whenever it is live-in per > > > DF_LR_IN, so the hoisted TLS call is kept after the DRAP save. > > > > > > PR target/126382 > > > > > > gcc/ChangeLog: > > > > > > * config/i386/i386-features.cc (ix86_emit_tls_call): Additional > > > check to see if DRAP register is live in basic block with > > > DF_LR_IN. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * gcc.target/i386/pr126382.c: New test. > > > > > > --- > > > > > > Hi, > > > > > > This patch fix the bug reported in > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=126382 > > > x86_cse uses DF_LIVE_IN to place the hoisted __tls_get_addr call, but > > > the DRAP register is missing there (no reaching def on entry), so the > > > call can land before the DRAP (%r10) save and clobber the by-value > > > argument > > > re-read through %r10. > > > > > > Fix: mark the DRAP register live when DF_LR_IN reports it, keeping > > > the TLS call after the DRAP save. > > > > > > Bootstrapped and regression tested on x86_64-linux. > > > > > > Thanks, > > > Reshma Roy > > > > > > gcc/config/i386/i386-features.cc | 7 +++++ > > > gcc/testsuite/gcc.target/i386/pr126382.c | 40 > > > ++++++++++++++++++++++++ > > > 2 files changed, 47 insertions(+) > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr126382.c > > > > > > diff --git a/gcc/config/i386/i386-features.cc > > > b/gcc/config/i386/i386-features.cc > > > index d65b6ce7672..2c0685a0863 100644 > > > --- a/gcc/config/i386/i386-features.cc > > > +++ b/gcc/config/i386/i386-features.cc > > > @@ -4357,6 +4357,13 @@ ix86_emit_tls_call (rtx tls_set, x86_cse_kind kind, > > basic_block bb, > > > && !fixed_regs[i] > > > && bitmap_bit_p (in, i)) > > > bitmap_set_bit (live_caller_saved_regs, i); > > > + if (df_live && crtl->drap_reg) > > > + { > > > + /* Check if DRAP is live in this BB with DF_LR_IN. */ > > > + i = REGNO (crtl->drap_reg); > > > + if (bitmap_bit_p (DF_LR_IN (bb), i)) > > > + bitmap_set_bit (live_caller_saved_regs, i); > > > + } > > > } > > > > DF_LR without any def to kill it will propagate "live" backward through > > essentially > > every block reachable from entry. So once > > crtl->drap_reg is set, won't bitmap_bit_p (DF_LR_IN (bb), i) end up > > true for every bb in the function? If that's the case, it would be simpler > > to drop the > > bitmap query entirely: > > > > if (df_live && crtl->drap_reg) > > { > > /* DRAP has no reaching definition at this point, so it's > > dropped from df_live's live-in set above. It is live > > for the whole function once assigned, so mark it here > > unconditionally rather than querying DF_LR_IN. */ > > bitmap_set_bit (live_caller_saved_regs, REGNO (crtl->drap_reg)); > > } > > > > Is there a path where crtl->drap_reg is set but DF_LR_IN(bb) is false for > > some bb in > > the function? > > We could find a test case where crtl->drap_reg is set but DF_LR_IN(bb) is > false for some bb. > Its value is copied into another register right in the entry block, and after > that DRAP has no further uses. > PS: The reference from rtl dump for block number is added in the comment. > > FLA_Hess_UT_blk_var5 (FLA_Obj A, int p, int q) > { // bb2 (entry): DRAP (%r10) > copied out to a register here, which is the last use of %r10 > long r = 0; > FLA_Obj_width (FLA_ONE); > // bb2 >> FLA_ONE > __tls_get_addr HOISTED into bb2 from bb7 > if (p) > { > // bb3 >> FLA_TWO > __tls_get_addr HOISTED into bb3 from bb6 > while (q--) > FLA_Obj_width (FLA_TWO); > r = FLA_Obj_length (A); > FLA_Obj_width (FLA_TWO); // bb6 > } > FLA_Obj_width (FLA_ONE); // bb7 > return r; > } > > If we print the DF_LR_IN for the blocks: > > bb=2 drap_reg=38 df_live=1 DF_LR_IN=1 DF_LR_OUT=0 > bb=3 drap_reg=38 df_live=1 DF_LR_IN=0 DF_LR_OUT=0 > > > Here crtl->drap_reg is set for the whole function, but at the TLS-call sites > DF_LR_IN(DRAP) > is 1 in bb2 and 0 in bb3.
Thanks for digging up the trace. Since DRAP's value gets copied out of %r10 in the entry block and %r10 has no further uses after that, DF_LR_IN(bb) for %r10 legitimately goes to 0 downstream (as in your bb3), so the per-block query is doing real work and isn't redundant with just checking crtl->drap_reg. The patch is OK with a comment improvement - could you fold the bb2/bb3 reasoning into the comment? "Check if DRAP is live in this BB with DF_LR_IN" doesn't hint at why DF_LR_IN is needed over just crtl->drap_reg. Something like: /* DRAP has no reaching definition at this point, so df_live drops it above. Its hard register can also go dead mid-function once copied elsewhere (e.g. right after the prologue), so query DF_LR_IN per-block rather than treating it as live whenever crtl->drap_reg is set. */ Uros.
