Alan Modra wrote: > However, the "o" constraint rejects any offset >= 0x7ff4 due to > rs6000_mode_dependent_address. This particular problem has been known > for a long time, but that's not the only problem with "o" (and also > the rs6000 "Y" constraint, a variant of "o"). A more subtle > requirement on constraints is that the relevant ones must match the > output of legitimize_reload_address. rs6000_legitimize_reload_address > for example can generate an address like > (plus (plus (reg x) (const_int high_part)) (const_int low_part)) > for stack slots and other large offset mems, pushing a reload for the > innermost "plus". So the final address will just be > (plus (reg y) (const_int low_part)) > an offset address, but the constraint sees that nested "plus" rtl. > Also, strict_memory_address_addr_space_p must *not* match the output > of legitimize_reload_address when reloads have been pushed. The > reason for this is that reload often makes multiple passes over insns, > and the modifications done by legitimize_reload_address are permanent. > On the second or subsequent passes, if that "(plus (plus ..))" address > is seen as valid, reload will leave it that way. So you lose the > reloads and are left with an invalid insn. > > First reload question: Is any of the above para incorrect?
This looks all good to me. As an aside, I'm currently looking into another problem related to offsettable memory operand triggered on AArch64: http://gcc.gnu.org/ml/gcc-patches/2012-06/msg01883.html After some investigation I'm now tending to think that the notion of an "offsettable" address in general is probably misguided, and should not be used at all in generic code, because there's just too many different variations on what exactly is required, depending on the particular situation. Instead, I think generic code should always construct the actual memory operand it wants to have considered, and then ask whether this is legitimate. For insn patterns, the target probably ought to use specific memory constraints adapted to the particular situation instead of relying on the generic 'o' constraint. > Now the "o" constraint is offsettable_nonstrict_memref_p, and the > rs6000 "Y" constraint currently calls memory_operand. The validity of > rs6000 offset addresses thus depends on the result of > rs6000_legitimate_address_p with "strict" false. Which means that > rs6000_legitimate_address_p ought to accept those "(plus (plus ..))" > addresses with "strict" false and reload_in_progress. Not really -- rs6000_legitimate_address_p really should only accept legitimate addresses. Only constraint matching needs to handle the intermediate forms that may occur temporarily during reload. As you noticed, this can cause problems when a constraint invokes a predcated function. This really ought to be avoided. Instead, the constraint needs to assume that it will only ever be called with a form of the address that has already passed the predicate, except as possibly modified by reload. > However, when I > tried that I ran into a gfortran testsuite segfault, cray_pointers_2. > Investigating that showed (set (mem:DI (pre_inc:SI 241)) (reg:DI 488)) > was getting two reload insns for pseudo 241 reversed. I saw > (set (reg:SI 8) (mem:SI (plus:SI (reg:SI 7) (const_int -16346)))) > (set (reg:SI 7) (plus:SI (reg:SI 1) (const_int 65536))) > Something to do with special treatment of pre_inc somewhere, no doubt, > and probably not too hard to fix, but at that point I decided I'd had > enough of reload. Not sure what's going on here, that needs further investigation ... > So instead this patch removes the unnecessary memory_operand call from > the "Y" constraint, and replaces most uses of the "o" constraint with > either "Y" or "wY", a new constraint like "Y" but for fprs. The idea > is that these constraints handle mems for just one type of reg, gpr or > fpr, and thus can properly handle the offset restrictions. "o" can't > do this because the mem mode does *not* guarantee the register type > being loaded/stored. For example, a DImode mem may well be used to > load/store a floating point reg, and a DFmode mem can be used with a > gpr (or two). This type of problem also makes it impossible to write > a version of rs6000_legitimate_offset_address_p that both allows ideal > code and is strict enough to be used with simple "m" constraints for > anything but 32-bit or smaller modes. What I've done there is to > relax rs6000_legitimate_offset_address_p when used in operand > predicates or by most mem validation functions, assuming best-case > regs are going to be used. If worst-case regs are used, then the > constraints ought to sort that out, forcing reloads. This makes sense to me. An address involving pseudos should be considered "legitimate" if there exists an assignment of hard registers that makes it strictly legitimate (not if *any* such assignment would be strictly legitimate). [ It might make sense in some cases to make the check stricter; for example if we know that an address would nearly always require a reload, we might choose to completely reject it if that actually increases performance. But that would be just performance tuning, not required for correctness ... ] > (This change is probably incomplete. I think I need to write more > secondary reload patterns. For example to handle a 64-bit mem with > an offset in the range 7ffc to 7fff used with 32-bit gprs. > Second reload question: Am I correct in assuming this is needed? The > case I'm thinking of is a pseudo that doesn't get a hard reg but has a > non-stack mem equiv.) In general, reload assumes it is possible to load *any* MEM with a legitimate address into *any* hard register that accepts its mode. All such cases that cannot be handled directly with an insn need a secondary reload. [ Some such cases may be hard to trigger. But I would prefer not relying on them actually never occurring ... ] Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE ulrich.weig...@de.ibm.com
