Re: [PATCH] Some middle-end improvements for bitfield handling (followup)

[Jakub Jelinek <jakub at redhat dot com>]

On Thu, Jul 01, 2004 at 06:58:50AM -0600, Roger Sayle wrote:
> If you're interested in yet another follow-up patch, I've two more
> suggestions.  The first is that it should be possible to tweak your
> code such that it works for "want_value".  In both the 1-bit XOR and
> the high-part addition cases, it's relatively simple to get the
> result of the assignment (use NULL_RTX as the target instead of
> to_rtx in the fnal expand_binop, and then shift/tweak "result" after
> the call to emit_move_insn), and still cheaper than extracting,
> modifying, masking and reinserting bitfields.

You're right.  I left want_value out because I thought that !want_value
is far more common for bitfields and wanted to optimize just the common
case.

> The second suggestion (or proposal) is that I believe that you don't
> need to check for TREE_THIS_VOLATILE(to), only TREE_SIDE_EFFECTS(to).
> Both these optimizations still execute a read/modify/write sequence
> to the volatile memory, exactly as they did before.  Hence any
> memory-mapped I/O device, or shared memory thread will see exactly
> the same sequence of memory accesses.
> 
> See the section in http://gcc.gnu.org/projects/optimize.html#volatile
> "Deficiencies of GCC's Optimizer: Volatile inhibits too many
> optimizations".

GCC normally generates 2 reads and 1 write for say:
volatile struct { unsigned int i : 3, j : 29; } b;
void test (unsigned int x)
{
  b.j += x;
}
        movl    b(%rip), %eax
        movl    b(%rip), %edx
        shrl    $3, %eax
        andl    $7, %edx
        addl    %edi, %eax
        sall    $3, %eax
        orl     %eax, %edx
        movl    %edx, b(%rip)
Without the TREE_THIS_VOLATILE check suddenly there would be just one
read and one write.  I was not sure if that behaviour change would be ok.

Well, I'm not sure whether what GCC used to do is ok either, consider
e.g.:
volatile struct { unsigned int i : 6, j : 11, k : 15; } b;
void test (unsigned int x)
{
  b.k += x;
}
        movzwl  b+2(%rip), %eax
        movl    b(%rip), %edx
        andl    $131071, %edx
        shrw    %ax
        movzwl  %ax, %eax
        addl    %edi, %eax
        sall    $17, %eax
        orl     %eax, %edx
        movl    %edx, b(%rip)
Note one 2 byte and one 4 byte read and one 4 byte store.
If the bitfields were in I/O, most probably the device wouldn't be
very happy about that.

	Jakub