Message ID | 014d01d9c63e$c36294b0$4a27be10$@nextmovesoftware.com |
---|---|
State | New |
Headers | show |
Series | Specify signed/unsigned/dontcare in calls to extract_bit_field_1. | expand |
On Thu, Aug 3, 2023 at 9:15 PM Roger Sayle <roger@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 <roger@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 > -- >
diff --git a/gcc/expmed.cc b/gcc/expmed.cc index fbd4ce2..b294eabb 100644 --- a/gcc/expmed.cc +++ b/gcc/expmed.cc @@ -1631,6 +1631,7 @@ extract_bit_field_as_subreg (machine_mode mode, rtx op0, } /* A subroutine of extract_bit_field, with the same arguments. + If UNSIGNEDP is -1, the result need not be sign or zero extended. If FALLBACK_P is true, fall back to extract_fixed_bit_field if we can find no other means of implementing the operation. if FALLBACK_P is false, return NULL instead. */ @@ -1933,7 +1934,8 @@ extract_integral_bit_field (rtx op0, opt_scalar_int_mode op0_mode, rtx result_part = extract_bit_field_1 (op0, MIN (BITS_PER_WORD, bitsize - i * BITS_PER_WORD), - bitnum + bit_offset, 1, target_part, + bitnum + bit_offset, + (unsignedp ? 1 : -1), target_part, mode, word_mode, reverse, fallback_p, NULL); gcc_assert (target_part); @@ -2187,6 +2189,7 @@ extract_fixed_bit_field (machine_mode tmode, rtx op0, /* Helper function for extract_fixed_bit_field, extracts the bit field always using MODE, which is the mode of OP0. + If UNSIGNEDP is -1, the result need not be sign or zero extended. The other arguments are as for extract_fixed_bit_field. */ static rtx @@ -2231,7 +2234,8 @@ extract_fixed_bit_field_1 (machine_mode tmode, rtx op0, scalar_int_mode mode, /* Unless the msb of the field used to be the msb when we shifted, mask out the upper bits. */ - if (GET_MODE_BITSIZE (mode) != bitnum + bitsize) + if (GET_MODE_BITSIZE (mode) != bitnum + bitsize + && unsignedp != -1) return expand_binop (new_mode, and_optab, op0, mask_rtx (new_mode, 0, bitsize, 0), target, 1, OPTAB_LIB_WIDEN); diff --git a/gcc/testsuite/gcc.target/i386/pr110717-2.c b/gcc/testsuite/gcc.target/i386/pr110717-2.c new file mode 100644 index 0000000..a3cb568 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr110717-2.c @@ -0,0 +1,20 @@ +/* PR target/110717 */ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +#ifdef __SIZEOF_INT128__ +#define type __int128 +#define N 59 +#else +#define type long long +#define N 27 +#endif + +struct S { type a : sizeof (type) * __CHAR_BIT__ - N; }; + +unsigned type bar (struct S *p) +{ + return p->a; +} + +/* { dg-final { scan-assembler-not "andl" } } */