This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] fix ice on comparison of aggregates with bitfield

Thanks for your review, Roger,

Roger Sayle wrote:
> > 	* gimplify.c (gimplify_scalar_mode_aggregate_compare): New function.
> This is OK for mainline with two changes.

> Firstly, in your new function gimplify_scalar_mode_aggregate_compare
> you should use fold_build1 and fold_build2 instead of build1 and
> build2 respectively.


> Now that fold can simplify the VIEW_CONVERT_EXPR at compile-time,
> your new code can easily be optimized to a compare of an integer
> constant at the tree-level.

 Understood. Thanks for the rationale.

> Secondly, could you add the testcase from the above post, "procedure P2"
> to the new GNAT testsuite.

 Certainly. It's really great to be able to do this now :)

> Could you please confirm that the compiler ICEs on the new testcase
> (i.e. it fails) without your patch, and passes with it?

 Things turned out to be a bit more involved ...

 The compiler still ICEs the same way without the patch, and the initial
 patch alone triggers a bootstrap failure: the stage1 compiler chokes while
 compiling sem_attr.adb for stage2.

 Attached is a simple patch suggestion to fix this. Combined with the
 initial patch adjusted to use fold_build as per your suggestion, the new
 compiler bootstraps fine, passes the initial test and has no new regression
 for languages=all,ada on i686-pc-linux-gnu.

 A reduced case for the boostrap failure is:

   function Scalar_Mode_Agg_Compare_Loop return Boolean is
      S : constant String (1 .. 4) := "ABCD";
      F : constant Natural := S'First;
      L : constant Natural := S'Last;
      for J in F .. L - 1 loop
	 if S (F .. F) = "X" or (J <= L - 2 and S (J .. J + 1) = "YY") then
	    return True;
	 end if;
      end loop;

      return False;

 The failure mode, compiling the code above with -O2 -gnatp, is:

   scalar_mode_agg_compare_loop.adb:5: error: missing definition
   for SSA_NAME: j.4_44 in statement:
   D.645_45 = VIEW_CONVERT_EXPR<UNSIGNED_16>("ABCD"[j.4_44 ...]{lb: 1 sz: 1});
   +===========================GNAT BUG DETECTED==============================+
   | 4.2.0 20060621 (experimental) (i686-pc-linux-gnu) verify_ssa failed      |
   | Error detected at scalar_mode_agg_compare_loop.adb:16:11                 |

 -fdump-tree-all reveals that ivopts has removed the def statement:

  j.0_7 = (scalar_mode_agg_compare_loop__L_1__T2b___XDLU_1__3) j_37;
  j.4_44 = (<unnamed type>) j.0_7;
  D.645_45 = VIEW_CONVERT_EXPR<UNSIGNED_16>("ABCD"[j.4_44 ...]{lb: 1 sz: 1});

  D.645_45 = VIEW_CONVERT_EXPR<UNSIGNED_16>("ABCD"[j.4_44 ...]{lb: 1 sz: 1});

 It seems to me it has done so because "for_each_index" currently
 doesn't visit the low bound expression for ARRAY_RANGE_REFs, despite
 what the head comment says:

   Access to an array: ARRAY_{RANGE_}REF (base, index).  In this case we also
   pass the pointer to the index to the callback.

 So ...

2006-06-23  Olivier Hainque  <>

	* gimplify.c (gimplify_scalar_mode_aggregate_compare): New function.
	(gimplify_expr): Use it for tcc_comparison of operands of non BLKmode
	aggregate types.

2006-06-23  Olivier Hainque  <>

	* tree-ssa-loop-im.c (for_each_index): Handle ARRAY_RANGE_REF as
	ARRAY_REF, thus have the callback called for the low bound expression.

2006-06-23  Olivier Hainque  <>

	* gnat.dg/scalar_mode_agg_compare_loop.adb: New test.
	* gnat.dg/scalar_mode_agg_compare.adb: New test.

As aforementioned, bootstrap and regression tests both fine on

OK to commit ?

Thanks in advance,



Attachment: scalar-mode-agg-compare-loop-part1.dif
Description: Text document

Attachment: scalar-mode-agg-compare-loop-part2.dif
Description: Text document

Attachment: scalar-mode-agg-compare-loop-tests.dif
Description: Text document

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]