This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, rs6000, testsuite] Fix PR target/64579, __TM_end __builtin_tend failed to return transactional state


On Tue, 2015-04-21 at 21:17 -0500, Segher Boessenkool wrote:
> On Tue, Apr 21, 2015 at 03:56:18PM -0500, Peter Bergner wrote:
> > This patch also fixes some issues I hit with the tabortdc[i] and
> > htm_m[ft]spr_<mode> patterns when used with -m32 -mpowerpc64.
> 
> Running the testsuite, or did you actually try to _use_ -m32 -mpowerpc64?  :-)

Not with the testsuite.  I had some simple unit tests that basically
just returned the CR/SPR and hit some ICEs.





> Maybe you can fold tabortdc with tabortwc now?  Use one UNSPEC name
> for both, :GPR and <wd>?

Wouldn't that change the tabortwc pattern to use DImode rather
than SImode when compiled with -m64 or -m32 -mpowerpc64?
I'm not sure we want that.



> > +	  case HTM_BUILTIN_TTEST: /* Alias for: tabortwci. 0,r0,0  */
> > +	    op[nopnds++] = GEN_INT (0);
> > +	    op[nopnds++] = gen_rtx_REG (SImode, 0);
> > +	    op[nopnds++] = GEN_INT (0);
> 
> Is that really r0, isn't that (0|rA)?  [Too lazy to read the docs myself
> right now, sorry.]

The ISA doc shows:

  tabortwci. TO,RA,SI

  a <- EXTS((RA)32:63)
  abort <- 0
  CR0 <- 0 || MSR(TS) || 0

  if a < EXTS(SI) & TO0 then abort <- 1
  if a > EXTS(SI) & TO1 then abort <- 1
  if a = EXTS(SI) & T02 then abort <- 1
  if a u< EXTS(SI) & TO3 then abort <- 1
  if a >u EXTS(SI) & TO4 then abort <- 1

  ...

Given that I'm passing in a zero TO value, the second and third
operands are don't care values, so I'm just using r0 and 0 as
random input values.  I'm only interested in extracting the
MSR's Transaction Status (TS) bits and placing them into CR0.



> > +		emit_insn (gen_movcc (subreg, cr));
> > +		emit_insn (gen_lshrsi3 (scratch2, scratch1, GEN_INT (28)));
> > +		emit_insn (gen_andsi3 (target, scratch2, GEN_INT (0xf)));
> > +	      }
> > +	  }
> 
> Don't we have helper functions/expanders to do these moves?  Yuck.

Heh, I looked.  The only helper pattern was the movcc pattern, but
that placed the CR into bits 32-35 of the register.  I needed the
shift to move it down into the low nibble and I use the "and", since
one of the move cr insns places two copies of the CR value into
bits 32-35 and 36-39.


> > -/* { dg-final { scan-assembler-times "tabortdc\\." 1 } } */
> > -/* { dg-final { scan-assembler-times "tabortdci\\." 1 } } */
> > +/* { dg-final { scan-assembler-times "tabortdc\\." 1 { target lp64 } } } */
> > +/* { dg-final { scan-assembler-times "tabortdci\\." 1 { target lp64 } } } */
> 
> This skips this test on -m32 -mpowerpc64, is that on purpose?

Ummm, not exactly.  :-)   Not that many people test that though.
I'll see if I can find a replacement for lp64 that covers that case.
If not, I'm not too torn up if we skip it for -m32 -mpowerpc64.


Peter




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]