Kenneth Zadeck <zad...@naturalbridge.com> writes:
>> From fold-const.c:
>>
>> @@ -13686,14 +13548,17 @@ fold_binary_loc (location_t loc,
>> break;
>> }
>>
>> - else if (TREE_INT_CST_HIGH (arg1) == signed_max_hi
>> - && TREE_INT_CST_LOW (arg1) == signed_max_lo
>> + else if (wi::eq_p (arg1, signed_max)
>> && TYPE_UNSIGNED (arg1_type)
>> + /* KENNY QUESTIONS THE CHECKING OF THE BITSIZE
>> + HERE. HE FEELS THAT THE PRECISION SHOULD BE
>> + CHECKED */
>> +
>> /* We will flip the signedness of the comparison operator
>> associated with the mode of arg1, so the sign bit is
>> specified by this mode. Check that arg1 is the signed
>> max associated with this sign bit. */
>> - && width == GET_MODE_BITSIZE (TYPE_MODE (arg1_type))
>> + && prec == GET_MODE_BITSIZE (TYPE_MODE (arg1_type))
>> /* signed_type does not work on pointer types. */
>> && INTEGRAL_TYPE_P (arg1_type))
>> {
>>
>> Looks like it should be resolved one way or the other before the merge.
> i am the one who asked the question.
OK, but this sort of thing should be handled separately from the wide-int
conversion, e.g. by a question to gcc@ or a patch to gcc-patches@ that
adds a "??? ..."-style comment.
>> From gcse.c:
>>
>> --- wide-int-base/gcc/gcc/gcse.c 2013-11-05 13:09:32.148376180 +0000
>> +++ wide-int/gcc/gcc/gcse.c 2013-11-05 13:07:28.431495118 +0000
>> @@ -1997,6 +1997,13 @@ prune_insertions_deletions (int n_elems)
>> bitmap_clear_bit (pre_delete_map[i], j);
>> }
>>
>> + if (dump_file)
>> + {
>> + dump_bitmap_vector (dump_file, "pre_insert_map", "", pre_insert_map,
>> n_edges);
>> + dump_bitmap_vector (dump_file, "pre_delete_map", "", pre_delete_map,
>> + last_basic_block);
>> + }
>> +
>> sbitmap_free (prune_exprs);
>> free (insertions);
>> free (deletions);
>>
>> This doesn't look related.
> when we were a/b tests with trunk we added this to trunk also. This this
> is useful, but it could also be removed.
>
>> --------------------
>>
>> From lcm.c:
>>
>> diff -udpr '--exclude=.svn' '--exclude=.pc' '--exclude=patches'
>> wide-int-base/gcc/gcc/lcm.c wide-int/gcc/gcc/lcm.c
>> --- wide-int-base/gcc/gcc/lcm.c 2013-08-22 09:00:23.068716382 +0100
>> +++ wide-int/gcc/gcc/lcm.c 2013-10-26 13:19:16.287277520 +0100
>> @@ -64,6 +64,7 @@ along with GCC; see the file COPYING3.
>> #include "sbitmap.h"
>> #include "dumpfile.h"
>>
>> +#define LCM_DEBUG_INFO 1
>> /* Edge based LCM routines. */
>> static void compute_antinout_edge (sbitmap *, sbitmap *, sbitmap *,
>> sbitmap *);
>> static void compute_earliest (struct edge_list *, int, sbitmap *, sbitmap
>> *,
>> @@ -106,6 +107,7 @@ compute_antinout_edge (sbitmap *antloc,
>> /* We want a maximal solution, so make an optimistic initialization of
>> ANTIN. */
>> bitmap_vector_ones (antin, last_basic_block);
>> + bitmap_vector_clear (antout, last_basic_block);
>>
>> /* Put every block on the worklist; this is necessary because of the
>> optimistic initialization of ANTIN above. */
>> @@ -432,6 +434,7 @@ pre_edge_lcm (int n_exprs, sbitmap *tran
>>
>> /* Allocate an extra element for the exit block in the laterin vector.
>> */
>> laterin = sbitmap_vector_alloc (last_basic_block + 1, n_exprs);
>> + bitmap_vector_clear (laterin, last_basic_block);
>> compute_laterin (edge_list, earliest, antloc, later, laterin);
>>
>> #ifdef LCM_DEBUG_INFO
> this is less so. it was added for debugging. But the problem was
> that the bitvectors were never initialized. it could be argued that
> in correct code that this would not matter unless you actually were
> looking the values for debugging which we were doing. I would
> certainly say that you could remove the define, but others should
> comment on removing the clears.
But here too I think this kind of change should be separate from the
wide-int conversion. If you'd like to see some of these changes made
then please submit a patch against mainline.
>> +/* Returns 1 if OP is an operand that is a CONST_WIDE_INT of mode
>> + MODE. This most likely is not as useful as
>> + const_scalar_int_operand since it does not accept CONST_INTs, but
>> + is here for consistancy. */
>> +int
>> +const_wide_int_operand (rtx op, enum machine_mode mode)
>> +{
>> + if (!CONST_WIDE_INT_P (op))
>> + return 0;
>> +
>> + return const_scalar_int_operand (op, mode);
>> +}
>>
>> Hmm, but have you found any use cases? Like you say in the comment,
>> it just seems wrong to require a CONST_WIDE_INT over a CONST_INT,
>> since that's a host-dependent choice. I think we should drop this
>> from the initial merge.
> no, but i have only converted the ppc to use this so far. however i
> am skeptical that there will be any clients. it could die and i would
> not be unhappy.
OK, well, since it's dead code at the moment and since IMO it's a bug
waiting to happen, I'm pretty strongly in favour of dropping it.
>> --------------------
>>
>> From rtl.c:
>>
>> +/* Write the wide constant X to OUTFILE. */
>> +
>> +void
>> +cwi_output_hex (FILE *outfile, const_rtx x)
>> +{
>> + int i = CWI_GET_NUM_ELEM (x);
>> + gcc_assert (i > 0);
>> + if (CWI_ELT (x, i-1) == 0)
>> + fprintf (outfile, "0x");
>> + fprintf (outfile, HOST_WIDE_INT_PRINT_HEX, CWI_ELT (x, --i));
>> + while (--i >= 0)
>> + fprintf (outfile, HOST_WIDE_INT_PRINT_PADDED_HEX, CWI_ELT (x, i));
>> +}
>>
>> Why only print "0x" if the top element is 0? Probably deserves a comment.
> I will add a comment.
Thanks.
Richard
