AMD General > -----Original Message----- > From: H.J. Lu <[email protected]> > Sent: Wednesday, July 29, 2026 4:56 AM > To: Uros Bizjak <[email protected]> > Cc: Roy, Reshma <[email protected]>; [email protected]; Kumar, > Venkataramanan <[email protected]>; Aloor, Raghesh > <[email protected]> > Subject: Re: [PATCH v2] 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 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 for the review and for committing the patch. > > 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) > > > > > > crtl->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.
