[PATCH] Fix PR32244 and some more (bitfield shifts and rotates wrong-code)
Richard Guenther
rguenther@suse.de
Fri Jan 25 17:17:00 GMT 2008
You should not look at dependent bugs ... because it may open a can
of worms. In the (non-regression) wrong-code bug PR32244 we shift
a bitfield left in the type of the bitfield (C doesn't promote the
operand), but we fail to truncate the result to the bitfield precision.
So far so easy. If you start playing with rotates (just to see if
we need to reduce them too), you notice that bitfield rotates
are completely hosed. Ugh.
First, we fold left-rotates to right-rotates based on the mode
bitsize, not the precision of the type. So, (x:40)<<r8 gets (x:40)>>r56.
Oops.
Then, we expand such rotate to rotate:DI, which of course is bogus for
rotates in a precision that doesn't match the mode. Oopsie.
Fixed along the following, splitting rotate handling from expand_shift
to expand_rotate.
All this doesn't seem to be a regression and bitfield rotates are
strange enough (but of course generated by fold, not the user, and
even more often now after Jakubs fixes), so I'll hold this off for 4.4
apart from reducing LSHIFT_EXPRs which I will post/test separately and
apply for 4.3.
Richard.
2008-01-25 Richard Guenther <rguenther@suse.de>
PR middle-end/32244
* expr.h (expand_rotate): Declare.
* expmed.c (expand_shift): Split out code expanding rotates ...
(expand_rotate): ... here. Do not use optabs for expansion of
rotates in a precision that doesn't match the mode.
* expr.c (expand_expr_real_1): Reduce results of LSHIFT_EXPR and all
rotates to their bitfield precision. Use expand_rotate for
expansion of rotates.
* fold-const.c (fold_binary): Use the types precision, not the
bitsize of the mode if folding rotate expressions.
* gcc.c-torture/execute/pr32244-1.c: New testcase.
* gcc.c-torture/execute/pr32244-2.c: Likewise.
Index: expr.c
===================================================================
*** expr.c (revision 131822)
--- expr.c (working copy)
*************** expand_expr_real_1 (tree exp, rtx target
*** 8920,8927 ****
target = 0;
op0 = expand_expr (TREE_OPERAND (exp, 0), subtarget,
VOIDmode, EXPAND_NORMAL);
! return expand_shift (code, mode, op0, TREE_OPERAND (exp, 1), target,
! unsignedp);
/* Could determine the answer when only additive constants differ. Also,
the addition of one can be handled by changing the condition. */
--- 8920,8936 ----
target = 0;
op0 = expand_expr (TREE_OPERAND (exp, 0), subtarget,
VOIDmode, EXPAND_NORMAL);
! if (code == RSHIFT_EXPR
! || code == LSHIFT_EXPR)
! temp = expand_shift (code, mode, op0, TREE_OPERAND (exp, 1), target,
! unsignedp);
! else
! temp = expand_rotate (code, mode, op0, TYPE_PRECISION (type),
! TREE_OPERAND (exp, 1), target);
! if (code != RSHIFT_EXPR)
! return REDUCE_BIT_FIELD (temp);
! else
! return temp;
/* Could determine the answer when only additive constants differ. Also,
the addition of one can be handled by changing the condition. */
Index: expmed.c
===================================================================
*** expmed.c (revision 131822)
--- expmed.c (working copy)
*************** expand_shift (enum tree_code code, enum
*** 2137,2146 ****
tree amount, rtx target, int unsignedp)
{
rtx op1, temp = 0;
! int left = (code == LSHIFT_EXPR || code == LROTATE_EXPR);
! int rotate = (code == LROTATE_EXPR || code == RROTATE_EXPR);
int try;
/* Previously detected shift-counts computed by NEGATE_EXPR
and shifted in the other direction; but that does not work
on all machines. */
--- 2137,2147 ----
tree amount, rtx target, int unsignedp)
{
rtx op1, temp = 0;
! int left = (code == LSHIFT_EXPR);
int try;
+ gcc_assert (code == LSHIFT_EXPR || code == RSHIFT_EXPR);
+
/* Previously detected shift-counts computed by NEGATE_EXPR
and shifted in the other direction; but that does not work
on all machines. */
*************** expand_shift (enum tree_code code, enum
*** 2193,2245 ****
else
methods = OPTAB_LIB_WIDEN;
! if (rotate)
! {
! /* Widening does not work for rotation. */
! if (methods == OPTAB_WIDEN)
! continue;
! else if (methods == OPTAB_LIB_WIDEN)
! {
! /* If we have been unable to open-code this by a rotation,
! do it as the IOR of two shifts. I.e., to rotate A
! by N bits, compute (A << N) | ((unsigned) A >> (C - N))
! where C is the bitsize of A.
!
! It is theoretically possible that the target machine might
! not be able to perform either shift and hence we would
! be making two libcalls rather than just the one for the
! shift (similarly if IOR could not be done). We will allow
! this extremely unlikely lossage to avoid complicating the
! code below. */
!
! rtx subtarget = target == shifted ? 0 : target;
! tree new_amount, other_amount;
! rtx temp1;
! tree type = TREE_TYPE (amount);
! if (GET_MODE (op1) != TYPE_MODE (type)
! && GET_MODE (op1) != VOIDmode)
! op1 = convert_to_mode (TYPE_MODE (type), op1, 1);
! new_amount = make_tree (type, op1);
! other_amount
! = fold_build2 (MINUS_EXPR, type,
! build_int_cst (type, GET_MODE_BITSIZE (mode)),
! new_amount);
!
! shifted = force_reg (mode, shifted);
!
! temp = expand_shift (left ? LSHIFT_EXPR : RSHIFT_EXPR,
! mode, shifted, new_amount, 0, 1);
! temp1 = expand_shift (left ? RSHIFT_EXPR : LSHIFT_EXPR,
! mode, shifted, other_amount, subtarget, 1);
! return expand_binop (mode, ior_optab, temp, temp1, target,
! unsignedp, methods);
! }
!
! temp = expand_binop (mode,
! left ? rotl_optab : rotr_optab,
! shifted, op1, target, unsignedp, methods);
! }
! else if (unsignedp)
temp = expand_binop (mode,
left ? ashl_optab : lshr_optab,
shifted, op1, target, unsignedp, methods);
--- 2194,2200 ----
else
methods = OPTAB_LIB_WIDEN;
! if (unsignedp)
temp = expand_binop (mode,
left ? ashl_optab : lshr_optab,
shifted, op1, target, unsignedp, methods);
*************** expand_shift (enum tree_code code, enum
*** 2247,2253 ****
/* Do arithmetic shifts.
Also, if we are going to widen the operand, we can just as well
use an arithmetic right-shift instead of a logical one. */
! if (temp == 0 && ! rotate
&& (! unsignedp || (! left && methods == OPTAB_WIDEN)))
{
enum optab_methods methods1 = methods;
--- 2202,2208 ----
/* Do arithmetic shifts.
Also, if we are going to widen the operand, we can just as well
use an arithmetic right-shift instead of a logical one. */
! if (temp == 0
&& (! unsignedp || (! left && methods == OPTAB_WIDEN)))
{
enum optab_methods methods1 = methods;
*************** expand_shift (enum tree_code code, enum
*** 2273,2278 ****
--- 2228,2329 ----
gcc_assert (temp);
return temp;
}
+
+ /* Output a rotate instruction for expression code CODE,
+ with SHIFTED being the rtx for the value with precision PRECISION
+ to rotate and AMOUNT the tree for the amount to rotate by.
+ Store the result in the rtx TARGET, if that is convenient.
+ Return the rtx for where the value is. */
+
+ rtx
+ expand_rotate (enum tree_code code, enum machine_mode mode,
+ rtx shifted, unsigned precision,
+ tree amount, rtx target)
+ {
+ rtx op1, temp = 0;
+ int left = (code == LROTATE_EXPR);
+ int try;
+
+ gcc_assert (code == LROTATE_EXPR || code == RROTATE_EXPR);
+ /* Previously detected shift-counts computed by NEGATE_EXPR
+ and shifted in the other direction; but that does not work
+ on all machines. */
+
+ op1 = expand_normal (amount);
+
+ if (SHIFT_COUNT_TRUNCATED)
+ {
+ if (GET_CODE (op1) == CONST_INT
+ && ((unsigned HOST_WIDE_INT) INTVAL (op1) >=
+ (unsigned HOST_WIDE_INT) GET_MODE_BITSIZE (mode)))
+ op1 = GEN_INT ((unsigned HOST_WIDE_INT) INTVAL (op1)
+ % GET_MODE_BITSIZE (mode));
+ else if (GET_CODE (op1) == SUBREG
+ && subreg_lowpart_p (op1))
+ op1 = SUBREG_REG (op1);
+ }
+
+ if (op1 == const0_rtx)
+ return shifted;
+
+ /* We can only use rotate instructions if we rotate in its mode precision. */
+ if (GET_MODE_PRECISION (mode) != precision)
+ try = 1;
+ else
+ try = 0;
+ for (; temp == 0 && try < 2; try++)
+ {
+ enum optab_methods methods;
+
+ if (try == 0)
+ methods = OPTAB_DIRECT;
+ else
+ methods = OPTAB_LIB_WIDEN;
+
+ if (methods == OPTAB_LIB_WIDEN)
+ {
+ /* If we have been unable to open-code this by a rotation,
+ do it as the IOR of two shifts. I.e., to rotate A
+ by N bits, compute (A << N) | ((unsigned) A >> (C - N))
+ where C is the bitsize of A.
+
+ It is theoretically possible that the target machine might
+ not be able to perform either shift and hence we would
+ be making two libcalls rather than just the one for the
+ shift (similarly if IOR could not be done). We will allow
+ this extremely unlikely lossage to avoid complicating the
+ code below. */
+
+ rtx subtarget = target == shifted ? 0 : target;
+ tree new_amount, other_amount;
+ rtx temp1;
+ tree type = TREE_TYPE (amount);
+ if (GET_MODE (op1) != TYPE_MODE (type)
+ && GET_MODE (op1) != VOIDmode)
+ op1 = convert_to_mode (TYPE_MODE (type), op1, 1);
+ new_amount = make_tree (type, op1);
+ other_amount
+ = fold_build2 (MINUS_EXPR, type,
+ build_int_cst (type, precision),
+ new_amount);
+
+ shifted = force_reg (mode, shifted);
+
+ temp = expand_shift (left ? LSHIFT_EXPR : RSHIFT_EXPR,
+ mode, shifted, new_amount, 0, 1);
+ temp1 = expand_shift (left ? RSHIFT_EXPR : LSHIFT_EXPR,
+ mode, shifted, other_amount, subtarget, 1);
+ return expand_binop (mode, ior_optab, temp, temp1,
+ target, 1, methods);
+ }
+
+ temp = expand_binop (mode, left ? rotl_optab : rotr_optab,
+ shifted, op1, target, 1, methods);
+ }
+
+ gcc_assert (temp);
+ return temp;
+ }
enum alg_code {
alg_unknown,
Index: fold-const.c
===================================================================
*** fold-const.c (revision 131822)
--- fold-const.c (working copy)
*************** fold_binary (enum tree_code code, tree t
*** 11627,11633 ****
if (code == LROTATE_EXPR && TREE_CODE (arg1) == INTEGER_CST)
{
tree tem = build_int_cst (TREE_TYPE (arg1),
! GET_MODE_BITSIZE (TYPE_MODE (type)));
tem = const_binop (MINUS_EXPR, tem, arg1, 0);
return fold_build2 (RROTATE_EXPR, type, op0, tem);
}
--- 11627,11633 ----
if (code == LROTATE_EXPR && TREE_CODE (arg1) == INTEGER_CST)
{
tree tem = build_int_cst (TREE_TYPE (arg1),
! TYPE_PRECISION (type));
tem = const_binop (MINUS_EXPR, tem, arg1, 0);
return fold_build2 (RROTATE_EXPR, type, op0, tem);
}
*************** fold_binary (enum tree_code code, tree t
*** 11646,11653 ****
fold_build2 (code, type,
TREE_OPERAND (arg0, 1), arg1));
! /* Two consecutive rotates adding up to the width of the mode can
! be ignored. */
if (code == RROTATE_EXPR && TREE_CODE (arg1) == INTEGER_CST
&& TREE_CODE (arg0) == RROTATE_EXPR
&& TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST
--- 11646,11653 ----
fold_build2 (code, type,
TREE_OPERAND (arg0, 1), arg1));
! /* Two consecutive rotates adding up to the precision of the
! type can be ignored. */
if (code == RROTATE_EXPR && TREE_CODE (arg1) == INTEGER_CST
&& TREE_CODE (arg0) == RROTATE_EXPR
&& TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST
*************** fold_binary (enum tree_code code, tree t
*** 11655,11661 ****
&& TREE_INT_CST_HIGH (TREE_OPERAND (arg0, 1)) == 0
&& ((TREE_INT_CST_LOW (arg1)
+ TREE_INT_CST_LOW (TREE_OPERAND (arg0, 1)))
! == (unsigned int) GET_MODE_BITSIZE (TYPE_MODE (type))))
return TREE_OPERAND (arg0, 0);
/* Fold (X & C2) << C1 into (X << C1) & (C2 << C1)
--- 11655,11661 ----
&& TREE_INT_CST_HIGH (TREE_OPERAND (arg0, 1)) == 0
&& ((TREE_INT_CST_LOW (arg1)
+ TREE_INT_CST_LOW (TREE_OPERAND (arg0, 1)))
! == (unsigned int) TYPE_PRECISION (type)))
return TREE_OPERAND (arg0, 0);
/* Fold (X & C2) << C1 into (X << C1) & (C2 << C1)
Index: testsuite/gcc.c-torture/execute/pr32244-1.c
===================================================================
*** testsuite/gcc.c-torture/execute/pr32244-1.c (revision 0)
--- testsuite/gcc.c-torture/execute/pr32244-1.c (revision 0)
***************
*** 0 ****
--- 1,22 ----
+ struct foo
+ {
+ unsigned long long b:40;
+ } x;
+
+ extern void abort (void);
+
+ void test1(unsigned long long res)
+ {
+ /* Build a rotate expression on a 40 bit argument. */
+ if ((x.b<<8) + (x.b>>32) != res)
+ abort ();
+ }
+
+ int main()
+ {
+ x.b = 0x0100000001;
+ test1(0x0000000101);
+ x.b = 0x0100000000;
+ test1(0x0000000001);
+ return 0;
+ }
Index: testsuite/gcc.c-torture/execute/pr32244-2.c
===================================================================
*** testsuite/gcc.c-torture/execute/pr32244-2.c (revision 0)
--- testsuite/gcc.c-torture/execute/pr32244-2.c (revision 0)
***************
*** 0 ****
--- 1,20 ----
+ struct foo
+ {
+ unsigned long long b:40;
+ } x;
+
+ extern void abort (void);
+
+ void test1(unsigned long long res)
+ {
+ /* The shift is carried out in 40 bit precision. */
+ if (x.b<<32 != res)
+ abort ();
+ }
+
+ int main()
+ {
+ x.b = 0x0100;
+ test1(0);
+ return 0;
+ }
Index: expr.h
===================================================================
--- expr.h (revision 131822)
+++ expr.h (working copy)
@@ -591,6 +591,8 @@ extern void fixup_tail_calls (void);
#ifdef TREE_CODE
extern rtx expand_shift (enum tree_code, enum machine_mode, rtx, tree, rtx,
int);
+extern rtx expand_rotate (enum tree_code, enum machine_mode, rtx, unsigned,
+ tree, rtx);
extern rtx expand_divmod (int, enum tree_code, enum machine_mode, rtx, rtx,
rtx, int);
#endif
More information about the Gcc-patches
mailing list