[RFC][PATCH] RL78 - Add predicates to reduce code bloat when accessing volatile memory.

[RFC][PATCH] RL78 - Add predicates to reduce code bloat when accessing volatile memory.

[Richard Hulme]

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(-)

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/>.
  \f
-(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

[PING][RFC][PATCH] RL78 - Add predicates to reduce code bloat when accessing volatile memory.

[Richard Hulme]

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(-)

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/>.
  \f
-(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.

[DJ Delorie]

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.

[Andrew Pinski]

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

Re: [PING][RFC][PATCH] RL78 - Add predicates to reduce code bloat when accessing volatile memory.

[Richard Hulme]

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.

Re: [PING][RFC][PATCH] RL78 - Add predicates to reduce code bloat when accessing volatile memory.

[DJ Delorie]

> 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.