Re: [PING][RFC][PATCH] RL78 - Add predicates to reduce code bloat when accessing volatile memory.
On 09/05/14 21:20, DJ Delorie wrote: The key to the msp430 change is that I reviewed every pattern that used the predicates, and only changed the ones where the pattern was known to be volatile-safe. For the RL78, the devirtualizer may make a pattern non-volatile-safe, and many patterns are macros which are not always volatile-safe. So, just changing the predicates is probably not enough. Did you review every pattern that used the changed predicates, to ensure that they're each volatile safe? Hi, Well, I've done my best to confirm that it doesn't break anything. I've looked over the patterns as best I could and looked at the code being generated (obviously not every pattern is being used), which didn't show anything that seemed wrong, and I've had no issues with running the generated code. My understanding of 'volatile' is that even the resulting code I gave as an example is not actually volatile-safe: mova, !240 ora, #32 mov!240, a as the value stored in address 240 could change just before and/or just after the 'or', and get overwritten by the second move. Given that the 'bsf' and 'bcf' instructions (bit set/bit clear) have not been included in any patterns, that is as volatile-safe as it can get, isn't it? As I understand it, one of the reasons for having the virtual pass was to make the instruction set appear completely orthogonal. Why then, when every instruction can use every virtual register in any addressing mode, does it think that: movwr10, #240 movwax, r10 movwhl, ax mova, [hl] ora, #32 mov[hl], a is more volatile-safe? Unless I'm missing something (which I'm quite prepared to accept) it's not less volatile-safe but it's certainly more bloated, and makes further optimizations harder. Richard.
Re: [PING][RFC][PATCH] RL78 - Add predicates to reduce code bloat when accessing volatile memory.
My understanding of 'volatile' is that even the resulting code I gave as an example is not actually volatile-safe: mova, !240 ora, #32 mov!240, a as the value stored in address 240 could change just before and/or just after the 'or', and get overwritten by the second move. That's not volatile, that's atomic. Volatile would mean, for example, that if the C source showed one reference to the variable, the assembler would have exactly one reference to the variable. If, for example, you had an SImode integer, some patterns might read the value more than once, which would violate the volatility. Those patterns must not use the new predicates. As I understand it, one of the reasons for having the virtual pass was to make the instruction set appear completely orthogonal. The reason is that the RL78 ISA is too unorthogonal for gcc to handle at all. I tried, it didn't work. Why then, when every instruction can use every virtual register in any addressing mode, does it think that: movwr10, #240 movwax, r10 movwhl, ax mova, [hl] ora, #32 mov[hl], a is more volatile-safe? Again, volatile != atomic. As long as the assembler references the variables exactly the same number of times as the C source says to, it's volatile-safe.
[PING][RFC][PATCH] RL78 - Add predicates to reduce code bloat when accessing volatile memory.
http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01108.html Original Message Subject: [RFC][PATCH] RL78 - Add predicates to reduce code bloat when accessing volatile memory. Date: Fri, 18 Apr 2014 21:15:01 +0200 From: Richard Hulme pepe...@yahoo.com To: GCC Patches gcc-patches@gcc.gnu.org Hi, This patch adds predicates (taken and renamed from the MSP430 backend) to allow more efficient code generation when accessing volatile memory turning this (for example): movwr10, #240 movwax, r10 movwhl, ax mov a, [hl] or a, #32 mov [hl], a into this: mov a, !240 or a, #32 mov !240, a Regards, Richard. 2014-04-18 Richard Hulme pepe...@yahoo.com * config/rl78/predicates.md (rl78_volatile_memory_operand): New (rl78_general_operand): New (rl78_nonimmediate_operand): New (rl78_any_operand): Now includes volatile memory (rl78_nonfar_operand): Likewise (rl78_nonfar_nonimm_operand): Likewise --- gcc/config/rl78/predicates.md | 26 +++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/gcc/config/rl78/predicates.md b/gcc/config/rl78/predicates.md index e564f43..29e3922 100644 --- a/gcc/config/rl78/predicates.md +++ b/gcc/config/rl78/predicates.md @@ -18,18 +18,38 @@ ;; along with GCC; see the file COPYING3. If not see ;; http://www.gnu.org/licenses/. -(define_predicate rl78_any_operand +(define_predicate rl78_volatile_memory_operand + (and (match_code mem) + (match_test (memory_address_addr_space_p (GET_MODE (op), XEXP (op, 0), MEM_ADDR_SPACE (op) +) + +; TRUE for any valid general operand. We do this because +; general_operand refuses to match volatile memory refs. + +(define_predicate rl78_general_operand (ior (match_operand 0 general_operand) + (match_operand 0 rl78_volatile_memory_operand)) +) + +; Likewise for nonimmediate_operand. + +(define_predicate rl78_nonimmediate_operand + (ior (match_operand 0 nonimmediate_operand) + (match_operand 0 rl78_volatile_memory_operand)) +) + +(define_predicate rl78_any_operand + (ior (match_operand 0 rl78_general_operand) (match_code mem,const_int,const_double,reg)) ) (define_predicate rl78_nonfar_operand - (and (match_operand 0 general_operand) + (and (match_operand 0 rl78_general_operand) (not (match_test rl78_far_p (op ) (define_predicate rl78_nonfar_nonimm_operand - (and (match_operand 0 nonimmediate_operand) + (and (match_operand 0 rl78_nonimmediate_operand) (not (match_test rl78_far_p (op ) -- 1.7.9.5
Re: [PING][RFC][PATCH] RL78 - Add predicates to reduce code bloat when accessing volatile memory.
The key to the msp430 change is that I reviewed every pattern that used the predicates, and only changed the ones where the pattern was known to be volatile-safe. For the RL78, the devirtualizer may make a pattern non-volatile-safe, and many patterns are macros which are not always volatile-safe. So, just changing the predicates is probably not enough. Did you review every pattern that used the changed predicates, to ensure that they're each volatile safe?
Re: [PING][RFC][PATCH] RL78 - Add predicates to reduce code bloat when accessing volatile memory.
On Fri, May 9, 2014 at 12:20 PM, DJ Delorie d...@redhat.com wrote:
The key to the msp430 change is that I reviewed every pattern that
used the predicates, and only changed the ones where the pattern was
known to be volatile-safe. For the RL78, the devirtualizer may make a
pattern non-volatile-safe, and many patterns are macros which are not
always volatile-safe.
So, just changing the predicates is probably not enough. Did you
review every pattern that used the changed predicates, to ensure that
they're each volatile safe?
More than that, did you review how combine handles volatile memory to
understand why combine does handle volatile memory?
Here is a testcase which combine miscompiles when I tried to have
combine handle volatile:
typedef unsigned long long uint64_t;
typedef unsigned int uint32_t;
union a
{
uint64_t b;
uint32_t c[2];
};
void f(int *r, volatile uint64_t *d)
{
union a e;
e.b = *d;
*r = e.c[1];
}
Thanks,
Andrew Pinski
[RFC][PATCH] RL78 - Add predicates to reduce code bloat when accessing volatile memory.
Hi, This patch adds predicates (taken and renamed from the MSP430 backend) to allow more efficient code generation when accessing volatile memory turning this (for example): movwr10, #240 movwax, r10 movwhl, ax mov a, [hl] or a, #32 mov [hl], a into this: mov a, !240 or a, #32 mov !240, a Regards, Richard. 2014-04-18 Richard Hulme pepe...@yahoo.com * config/rl78/predicates.md (rl78_volatile_memory_operand): New (rl78_general_operand): New (rl78_nonimmediate_operand): New (rl78_any_operand): Now includes volatile memory (rl78_nonfar_operand): Likewise (rl78_nonfar_nonimm_operand): Likewise --- gcc/config/rl78/predicates.md | 26 +++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/gcc/config/rl78/predicates.md b/gcc/config/rl78/predicates.md index e564f43..29e3922 100644 --- a/gcc/config/rl78/predicates.md +++ b/gcc/config/rl78/predicates.md @@ -18,18 +18,38 @@ ;; along with GCC; see the file COPYING3. If not see ;; http://www.gnu.org/licenses/. -(define_predicate rl78_any_operand +(define_predicate rl78_volatile_memory_operand + (and (match_code mem) + (match_test (memory_address_addr_space_p (GET_MODE (op), XEXP (op, 0), MEM_ADDR_SPACE (op) +) + +; TRUE for any valid general operand. We do this because +; general_operand refuses to match volatile memory refs. + +(define_predicate rl78_general_operand (ior (match_operand 0 general_operand) + (match_operand 0 rl78_volatile_memory_operand)) +) + +; Likewise for nonimmediate_operand. + +(define_predicate rl78_nonimmediate_operand + (ior (match_operand 0 nonimmediate_operand) + (match_operand 0 rl78_volatile_memory_operand)) +) + +(define_predicate rl78_any_operand + (ior (match_operand 0 rl78_general_operand) (match_code mem,const_int,const_double,reg)) ) (define_predicate rl78_nonfar_operand - (and (match_operand 0 general_operand) + (and (match_operand 0 rl78_general_operand) (not (match_test rl78_far_p (op ) (define_predicate rl78_nonfar_nonimm_operand - (and (match_operand 0 nonimmediate_operand) + (and (match_operand 0 rl78_nonimmediate_operand) (not (match_test rl78_far_p (op ) -- 1.7.9.5
