Message ID | CAFULd4bmNZZAzJmJdW6SyRj4k-OteJkMnpCf9-p7dF624drXfg@mail.gmail.com |
---|---|
State | New |
Headers | show |
On 07/20/2011 09:41 AM, Uros Bizjak wrote: > * config/i386/i386.c (ix86_decompose_address): Also allow promoted > paradoxical subregs in base and PLUS chains. Allow only paradoxical > subregs and subregs of DImode hard registers in subregs of index. > (ix86_legitimate_address_p): Allow subregs of base and index to span > more than a word. Assert that subregs of base and index satisfy > SUBREG_PROMOTED_UNSIGNED_P or register_no_elim_operand predicates. The patch looks good. It seems like one other check is required: that the mode of base and index are the same. We've been accepting either SImode or DImode. As long as they're both the same we get either mov (%rax, %rdx, 4), %ebx mov (%eax, %edx, 4), %ebx which are both valid insns (without and with addr32 prefix). Whereas mov (%rax, %edx, 4), %ebx will correctly be rejected by the assembler. r~
> Another improvement is introduction of SUBREG_PROMOTED_UNSIGNED_P > predicate in the same area to handle paradoxical SUBREGs. When SUBREG > satisfies this predicate, the compiler guarantees, that excess bits > are zero (see the documentation). This is exaclty what we want in > registers that form the address. In theory, we can allow registers of > any integer mode here, but in practice only DImode subreg of SImode > register is important. Note that SUBREG_PROMOTED_UNSIGNED_P wasn't designed for paradoxical subregs, but for regular subregs (typically of word-sized objects). You should check that the ones created for x32 (because of POINTERS_EXTEND_UNSIGNED I guess) are legitimate.
On Wed, Jul 20, 2011 at 8:04 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> Another improvement is introduction of SUBREG_PROMOTED_UNSIGNED_P >> predicate in the same area to handle paradoxical SUBREGs. When SUBREG >> satisfies this predicate, the compiler guarantees, that excess bits >> are zero (see the documentation). This is exaclty what we want in >> registers that form the address. In theory, we can allow registers of >> any integer mode here, but in practice only DImode subreg of SImode >> register is important. > > Note that SUBREG_PROMOTED_UNSIGNED_P wasn't designed for paradoxical subregs, > but for regular subregs (typically of word-sized objects). You should check > that the ones created for x32 (because of POINTERS_EXTEND_UNSIGNED I guess) > are legitimate. Hm, the documentation says: Paradoxical subregs When M1 is strictly wider than M2, the `subreg' expression is called "paradoxical". The canonical test for this class of `subreg' is: GET_MODE_SIZE (M1) > GET_MODE_SIZE (M2) Paradoxical `subreg's can be used as both lvalues and rvalues. When used as an lvalue, the low-order bits of the source value are stored in REG and the high-order bits are discarded. When used as an rvalue, the low-order bits of the `subreg' are taken from REG while the high-order bits may or may not be defined. The high-order bits of rvalues are in the following circumstances: * `subreg's of `mem' When M2 is smaller than a word, the macro `LOAD_EXTEND_OP', can control how the high-order bits are defined. * `subreg' of `reg's The upper bits are defined when `SUBREG_PROMOTED_VAR_P' is true. `SUBREG_PROMOTED_UNSIGNED_P' describes what the upper bits hold. Such subregs usually represent local variables, register variables and parameter pseudo variables that have been promoted to a wider mode. I have looked at example in rs6000.c, the only target that uses SUBREG_PROMOTED_UNSIGNED_P. Looking at other sources, S_P_U_P is used in conjunction with SUBREG_PROMOTED_VAR. It looks to me that using the combination should be OK to determine if subreg is correct. Uros.
Index: i386.c =================================================================== --- i386.c (revision 176512) +++ i386.c (working copy) @@ -11089,8 +11089,10 @@ ix86_decompose_address (rtx addr, struct base = addr; else if (GET_CODE (addr) == SUBREG) { - /* Allow only subregs of DImode hard regs. */ - if (register_no_elim_operand (SUBREG_REG (addr), DImode)) + /* Allow only promoted paradoxical subregs + or subregs of DImode hard regs. */ + if (SUBREG_PROMOTED_UNSIGNED_P (addr) + || register_no_elim_operand (SUBREG_REG (addr), DImode)) base = addr; else return 0; @@ -11148,8 +11150,10 @@ ix86_decompose_address (rtx addr, struct break; case SUBREG: - /* Allow only subregs of DImode hard regs in PLUS chains. */ - if (!register_no_elim_operand (SUBREG_REG (op), DImode)) + /* Allow only promoted paradoxical subregs + or subregs of DImode hard regs in PLUS chains. */ + if (!(SUBREG_PROMOTED_UNSIGNED_P (op) + || register_no_elim_operand (SUBREG_REG (op), DImode))) return 0; /* FALLTHRU */ @@ -11197,6 +11201,18 @@ ix86_decompose_address (rtx addr, struct else disp = addr; /* displacement */ + if (index) + { + if (REG_P (index)) + ; + /* Allow only promoted paradoxical subregs + or subregs of DImode hard regs. */ + else if (GET_CODE (index) == SUBREG + && !(SUBREG_PROMOTED_UNSIGNED_P (index) + || register_no_elim_operand (SUBREG_REG (index), DImode))) + return 0; + } + /* Extract the integral value of scale. */ if (scale_rtx) { @@ -11630,11 +11646,7 @@ ix86_legitimate_address_p (enum machine_ disp = parts.disp; scale = parts.scale; - /* Validate base register. - - Don't allow SUBREG's that span more than a word here. It can lead to spill - failures when the base is one word out of a two word structure, which is - represented internally as a DImode int. */ + /* Validate base register. */ if (base) { @@ -11642,11 +11654,12 @@ ix86_legitimate_address_p (enum machine_ if (REG_P (base)) reg = base; - else if (GET_CODE (base) == SUBREG - && REG_P (SUBREG_REG (base)) - && GET_MODE_SIZE (GET_MODE (SUBREG_REG (base))) - <= UNITS_PER_WORD) - reg = SUBREG_REG (base); + else if (GET_CODE (base) == SUBREG && REG_P (SUBREG_REG (base))) + { + reg = SUBREG_REG (base); + gcc_assert (SUBREG_PROMOTED_UNSIGNED_P (base) + || register_no_elim_operand (reg, DImode)); + } else /* Base is not a register. */ return false; @@ -11660,9 +11673,7 @@ ix86_legitimate_address_p (enum machine_ return false; } - /* Validate index register. - - Don't allow SUBREG's that span more than a word here -- same as above. */ + /* Validate index register. */ if (index) { @@ -11670,11 +11681,12 @@ ix86_legitimate_address_p (enum machine_ if (REG_P (index)) reg = index; - else if (GET_CODE (index) == SUBREG - && REG_P (SUBREG_REG (index)) - && GET_MODE_SIZE (GET_MODE (SUBREG_REG (index))) - <= UNITS_PER_WORD) - reg = SUBREG_REG (index); + else if (GET_CODE (index) == SUBREG && REG_P (SUBREG_REG (index))) + { + reg = SUBREG_REG (index); + gcc_assert (SUBREG_PROMOTED_UNSIGNED_P (index) + || register_no_elim_operand (reg, DImode)); + } else /* Index is not a register. */ return false;