Message ID | 536D259D.9080404@yahoo.com |
---|---|
State | New |
Headers | show |
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?
On Fri, May 9, 2014 at 12:20 PM, DJ Delorie <dj@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
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: mov a, !240 or a, #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: movw r10, #240 movw ax, r10 movw hl, ax mov a, [hl] or a, #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.
> My understanding of 'volatile' is that even the resulting code I gave as > an example is not actually volatile-safe: > > mov a, !240 > or a, #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: > > movw r10, #240 > movw ax, r10 > movw hl, ax > mov a, [hl] > or a, #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.
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)"))) )
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 <peper03@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): movw r10, #240 movw ax, r10 movw hl, 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 <peper03@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(-)