diff mbox

Fix wrong code with VCE to bit-field type at -O

Message ID 1629803.uCIxAQNden@polaris
State New
Headers show

Commit Message

Eric Botcazou Feb. 17, 2014, 10:27 a.m. UTC
> There is nothing obvious I think, i.e. that's debatable.  I agree that a VCE
> from a 32-bit object to a 32-bit integer with 24-bit precision should not
> clear the upper 8 bits (so the REDUCE_BIT_FIELD part of my patch is wrong).
> But here we have a VCE from a 24-bit object to a 32-bit integer with 24-bit
> precision which reads *more bits* than the size of the source type; that I
> think is plain wrong and is fixed by the bit-field extraction in the patch.

Revised patch along these lines attached.  Although I agree that it's a bit of 
a kludge, it's quite localized and plausible IMO.


	* expr.c (expand_expr_real_1) <case VIEW_CONVERT_EXPR>: For a bit-field
	destination type, extract exactly the number of valid bits if the source
	type isn't integral or has a different precision.

Comments

Richard Biener Feb. 17, 2014, 1:34 p.m. UTC | #1
On Mon, Feb 17, 2014 at 11:27 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> There is nothing obvious I think, i.e. that's debatable.  I agree that a VCE
>> from a 32-bit object to a 32-bit integer with 24-bit precision should not
>> clear the upper 8 bits (so the REDUCE_BIT_FIELD part of my patch is wrong).
>> But here we have a VCE from a 24-bit object to a 32-bit integer with 24-bit
>> precision which reads *more bits* than the size of the source type; that I
>> think is plain wrong and is fixed by the bit-field extraction in the patch.
>
> Revised patch along these lines attached.  Although I agree that it's a bit of
> a kludge, it's quite localized and plausible IMO.

Woudln't it be better to do this in the series of "conversions", that is
inside the preceeding if-statement?  (the integral type case using
convert_modes looks weird enough, so adding this kind-of "less"
weird one there looks sensible)

Ok with moving it there (before the else if (!MEM_P (op0))).  You
probably want to guard with INTEGRAL_TYPE_P (type) as well,
not only GET_MODE (op0) != mode - just to prepare for weird
stuff like a vector-type where TYPE_PRECISION means sth else.

Thanks,
Richard.



>
>         * expr.c (expand_expr_real_1) <case VIEW_CONVERT_EXPR>: For a bit-field
>         destination type, extract exactly the number of valid bits if the source
>         type isn't integral or has a different precision.
>
>
> --
> Eric Botcazou
diff mbox

Patch

Index: expr.c
===================================================================
--- expr.c	(revision 207796)
+++ expr.c	(working copy)
@@ -10458,15 +10458,21 @@  expand_expr_real_1 (tree exp, rtx target
 	  op0 = target;
 	}
 
-      /* At this point, OP0 is in the correct mode.  If the output type is
+      /* If OP0 is a MEM without the correct mode and we need to reduce it to
+	 a bit-field type, do an extraction.  Otherwise, we can read all bits
+	 of MODE but need to deal with the alignment.  If the output type is
 	 such that the operand is known to be aligned, indicate that it is.
-	 Otherwise, we need only be concerned about alignment for non-BLKmode
-	 results.  */
+	 Otherwise, we actually need only be concerned about alignment for
+	 non-BLKmode results.  */
       if (MEM_P (op0))
 	{
 	  enum insn_code icode;
 
-	  if (TYPE_ALIGN_OK (type))
+	  if (reduce_bit_field && GET_MODE (op0) != mode)
+	    return extract_bit_field (op0, TYPE_PRECISION (type), 0,
+				      TYPE_UNSIGNED (type), NULL_RTX,
+				      mode, mode);
+	  else if (TYPE_ALIGN_OK (type))
 	    {
 	      /* ??? Copying the MEM without substantially changing it might
 		 run afoul of the code handling volatile memory references in