On Thu, Aug 3, 2023 at 9:15 PM Roger Sayle <ro...@nextmovesoftware.com> wrote:
>
>
> This patch is inspired by Jakub's work on PR rtl-optimization/110717.
> The bitfield example described in comment #2, looks like:
>
> struct S { __int128 a : 69; };
> unsigned type bar (struct S *p) {
> return p->a;
> }
>
> which on x86_64 with -O2 currently generates:
>
> bar: movzbl 8(%rdi), %ecx
> movq (%rdi), %rax
> andl $31, %ecx
> movq %rcx, %rdx
> salq $59, %rdx
> sarq $59, %rdx
> ret
>
> The ANDL $31 is interesting... we first extract an unsigned 69-bit bitfield
> by masking/clearing the top bits of the most significant word, and then
> it gets sign-extended, by left shifting and arithmetic right shifting.
> Obviously, this bit-wise AND is redundant, for signed bit-fields, we don't
> require these bits to be cleared, if we're about to set them appropriately.
>
> This patch eliminates this redundancy in the middle-end, during RTL
> expansion, but extending the extract_bit_field APIs so that the integer
> UNSIGNEDP argument takes a special value; 0 indicates the field should
> be sign extended, 1 (any non-zero value) indicates the field should be
> zero extended, but -1 indicates a third option, that we don't care how
> or whether the field is extended. By passing and checking this sentinel
> value at the appropriate places we avoid the useless bit masking (on
> all targets).
>
> For the test case above, with this patch we now generate:
>
> bar: movzbl 8(%rdi), %ecx
> movq (%rdi), %rax
> movq %rcx, %rdx
> salq $59, %rdx
> sarq $59, %rdx
> ret
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> and make -k check, both with and without --target_board=unix{-m32}
> with no new failures. Ok for mainline?
OK.
Thanks,
Richard.
>
> 2023-08-03 Roger Sayle <ro...@nextmovesoftware.com>
>
> gcc/ChangeLog
> * expmed.cc (extract_bit_field_1): Document that an UNSIGNEDP
> value of -1 is equivalent to don't care.
> (extract_integral_bit_field): Indicate that we don't require
> the most significant word to be zero extended, if we're about
> to sign extend it.
> (extract_fixed_bit_field_1): Document that an UNSIGNEDP value
> of -1 is equivalent to don't care. Don't clear the most
> most significant bits with AND mask when UNSIGNEDP is -1.
>
> gcc/testsuite/ChangeLog
> * gcc.target/i386/pr110717-2.c: New test case.
>
>
> Thanks in advance,
> Roger
> --
>
