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] Fix PR87870: ppc64 generates poor code when loading constants into TImode vars


On Thu, Dec 13, 2018 at 10:59:36AM -0600, Peter Bergner wrote:
> On 11/16/18 5:29 PM, Segher Boessenkool wrote:
> > On Fri, Nov 16, 2018 at 04:26:18PM -0600, Peter Bergner wrote:
> >> However, when I made the change below, the length attribute seems a
> >> little off.  For *_64bit, we have a length of 4, but for *_32bit, we
> >> have a length of 32.  The "4" looks correct for both *_64bit and *_32bit
> >> if we're loading an easy_vector_constant into one of the vector regs.
> >> For loading a TImode constant into a GPR, then it could be anything from
> >> 8 bytes to 40bytes (loading 0xdeadbeefcafebabefacefeedbaaaaaad) for
> >> -m64.  Since TImode isn't supposrted in -m32 (yet?), who knows, probably
> >> it would be 16 bytes to ??? bytes.
> >>
> >> Should those sizes be updated too?  If so, what should they be?
> >> The smallest, average or worst case lengths?  I assume we could use
> >> another iterator to separate the vector lengths from the gpr lengths
> >> if we need to.
> > 
> > Worst case.  This is required for correctness.
> 
> Ok, I looked into this and the point where we must have correct length info
> is in final assembly generation, so very very late.

I'm not convinced this is true.  And problems with this only show up
with unusual code, so typically after releases :-(

Maybe we could make a mode where conditional jumps can jump only 128
bytes or similar, that would make testing much easier (problems will
show up much more often than with the 32kB max distance we have).

> For the alternatives
> I'm changing, we're loading into GPR regs and these alternatives are always
> split (split2), so these length values are never used/seen at final assembly
> time.

But some move instructions are created *after* split2.

> Given the above, I'm guessing we should probably go with the most common
> length value (ie, 8 for 64-bit and 16 for 32-bit)?  The following patch
> implements that.  Does this seem reasonable to you?

I do like the patch.  Let me sleep on it.


Segher


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