On Tue, Jul 28, 2026 at 7:28 PM Uros Bizjak <[email protected]> wrote: > > On Tue, Jul 28, 2026 at 1:26 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. > > OK.
Hi Reshma, I pushed it in for you. Thanks. > Thanks, > Uros. > > > > > --- > > > > Hi, > > > > The patch is updated based on the comment. > > > > Bootstrapped and regression tested on x86_64-linux. > > > > Thanks, > > Reshma Roy > > > > > -----Original Message----- > > > From: Uros Bizjak <[email protected]> > > > Sent: Tuesday, July 28, 2026 4:16 PM > > > 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 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. */ > > Updated the comments as per your suggestion. > > > > > > Uros. > > > > gcc/config/i386/i386-features.cc | 11 +++++++ > > gcc/testsuite/gcc.target/i386/pr126382.c | 40 ++++++++++++++++++++++++ > > 2 files changed, 51 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..65e1fe16143 100644 > > --- a/gcc/config/i386/i386-features.cc > > +++ b/gcc/config/i386/i386-features.cc > > @@ -4357,6 +4357,17 @@ 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) > > + { > > + /* 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. */ > > + i = REGNO (crtl->drap_reg); > > + if (bitmap_bit_p (DF_LR_IN (bb), i)) > > + bitmap_set_bit (live_caller_saved_regs, i); > > + } > > } > > > > if (bitmap_empty_p (live_caller_saved_regs)) > > diff --git a/gcc/testsuite/gcc.target/i386/pr126382.c > > b/gcc/testsuite/gcc.target/i386/pr126382.c > > new file mode 100644 > > index 00000000000..2ffe0074c21 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/i386/pr126382.c > > @@ -0,0 +1,40 @@ > > +/* { dg-do compile { target { *-*-linux* && lp64 } } } */ > > +/* { dg-options "-O3 -fPIC -march=x86-64-v4 > > -fno-asynchronous-unwind-tables -mtls-dialect=gnu" } */ > > +/* Keep labels and directives ('.cfi_startproc', '.cfi_endproc'). */ > > +/* { dg-final { check-function-bodies "**" "" "" { target "*-*-*" } > > {^\t?\.} } } */ > > + > > +/* > > +**func: > > +** pushq %rbp > > +** movq %rsp, %rbp > > +** pushq %r12 > > +** pushq %r10 > > +** leaq 16\(%rbp\), %r10 > > +** pushq %rbx > > +** movq %r10, %r12 > > +** subq \$8, %rsp > > +** data16 leaq FLA_ONE@tlsgd\(%rip\), %rdi > > +** .value 0x6666 > > +** rex64 > > +** call __tls_get_addr@PLT > > +**... > > +*/ > > + > > +typedef struct > > +{ > > + long n; > > + long m_inner; > > + long n_inner; > > + int base; > > + } FLA_Obj; > > +extern __thread FLA_Obj FLA_ONE, W12; > > +extern long FLA_Obj_length (FLA_Obj); > > +extern void FLA_Obj_width (FLA_Obj, ...); > > +void func (FLA_Obj A) > > +{ > > + while (FLA_Obj_length (A)) > > + FLA_Obj_width (FLA_ONE); > > + FLA_Obj_width (FLA_ONE, W12); > > +} > > + > > +/* { dg-final { scan-assembler-times "call\[ \t\]__tls_get_addr@PLT" 2 } } > > */ > > -- > > 2.34.1 > > -- H.J.
