Hi,

Segher Boessenkool <seg...@kernel.crashing.org> wrote:

On Sat, Oct 12, 2019 at 10:13:16PM +0100, Iain Sandoe wrote:
(this is a bug reported against Fortran, but actually is a generic insn
selection problem for m64 powerpc-darwin.  In fact, I’d say it renders
the 64b multilib unusable).

The solution proposed is Darwin-local (it's a long-standing bug and it
would be intended to back-port it).  However, I'd welcome views on it.

The current Darwin load/store lo_sum patterns have neither predicate nor
constraint.  This means that most parts of the backend, which rely on
recog() to validate the rtx, can produce invalid combinations/selections.

For 32bit cases this isn't a problem since we can load/store to unaligned
addresses using D-mode insns.

Can you?  -m32 -mpowerpc64?

In principle, this ought to be supported (there’s a pattern for the specific case in darwin.md) however, I don’t think it’s tested and there seem to be a number
of places where the conditional is TARGET_64BIT instead of
TARGET_POWERPC64
so my guess is that there would be some work to do to make it happen,

We did have a bug with this before, maybe
six years ago or so...  Alan, do you remember?  It required some assembler
work IIRC.

Conversely, for 64bit instructions that use DS mode, this can manifest as
assemble errors (for an assembler that checks the LO14 relocations), or as
crashes caused by wrong offsets (or worse, wrong content for the two LSBs).

Resulting in a different insn than intended.  Yeah.

Trying to find the right place to fix this has been tricky, there's
discussion in the PR trail.  There doesn't seem to be any way to deal with
this in legitimize_address (since most of the damage is done by the wide open
patterns that are in effect after the legitimizer has run).  Likewise,
extending the checks in legitimate_address_p() and mode_dependent_address
doesn't help if the constraint is not accurate in the end.

What we want to check for "Y" on Darwin is:
 - that the alignment of the Symbols' target is sufficient for DS mode
 - that the offset is suitable for DS mode.
(while looking through the Mach-O PIC unspecs).

Proposed:

1) Drop the Darwin-specific lo_sum patterns.  Actually, this produces an
immediate progression in quite a number of tests, we begin using the
movdi_internal64 patterns.  However there are also regressions caused by
instructions unable to satisfy their constraints.

And code quality regressions?  Or does it even improve?  (One can dream…)

The code quality for the testcases I made for this is good when we do the part below - because, unless we cater for the indirections, we generate quite a few
unnecessary loads for the PIC case.

2) To resolve this we need to extend the handling of the mem_operand_gpr to
allow looking through Mach-O PIC UNSPECs in the lo_sum cases.

- note, that rs6000_offsettable_memref_p () will not handle these so that
  would return early, producing the issue with unsatisfiable constraints.

 - I do wonder if that's also the case for some non-Darwin lo_sum cases.

Not sure what exactly you mean here?  Non-Darwin doesn't have issues with
not looking through Darwin-specific unspecs, anywhere, so you mean
something more general no doubt -- but what?

LO_SUM appears to be handled in the case that it refers to a constant pool
address, but I wonder if there are any other circumstances that it could be
relevant.

(some things might be hard to detect, since the code will generally fall
back to doing " la  Rx xxx@l ; ld Ry 0(Rx)" so it won't fail - just be
less efficient than it could be).

Oh, you are saying mem_operand_gpr does not allow lo_sum *at all*?

No, I think it will let LO_SUM through for constant pool entries, I’m not sure
about any other case tho (perhaps there is no other case that matters).

The absence of something is much harder to reason about / test for than the
presence of something - but I am sure that you and Alan know what you
expect to be there.

+ /* If we don't know the alignment of the thing to which the symbol refers,
+     we assume optimistically it is "enough".
+     ??? maybe we should be pessimistic instead.  */
+  unsigned align = 0;

If you guess it is aligned enough but it isn't, the compile will fail.  Not
good.  OTOH, when don't we know the alignment?  Only for globals?  Does the
ABI guarantee alignment in that case?

We usually know the alignment for globals too - I’m not sure under what
circumstance there is no decl attached to the symbol_ref
There is code rs6000.c:7627 or so, that suggests that section anchors can
produce the effect, so that’s something I can look at once the basic problem
is solved.

I'll have another looke through this (esp. the generic part) when I'm fresh
awake (but not before coffee!).  Alan, can you have a look as well please?
thanks!
Iain

Reply via email to