This is the mail archive of the gcc-patches@gcc.gnu.org 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 PR54733 Optimize endian independent load/store


On Mon, May 19, 2014 at 11:30 AM, Thomas Preud'homme
<thomas.preudhomme@arm.com> wrote:
>> From: Richard Biener [mailto:richard.guenther@gmail.com]
>>
>> Oh, and what happens for
>>
>> unsigned foo (unsigned char *x)
>> {
>>   return x[0] << 24 | x[2] << 8 | x[3];
>> }
>>
>> ?  We could do an unsigned int load from x and zero byte 3
>> with an AND.  Enhancement for a followup, similar to also
>> considering vector types for the load (also I'm not sure
>> that uint64_type_node always has non-BLKmode for all
>> targets).
>
> I implemented this but it makes the statistics more difficult to read
> and thus the test more difficult to write. Indeed, when doing a full
> bswap many of the intermediate computations are partial bswap.
> I can get on with it by using a different string to notify of partial
> bswap than full bswap and test the partial bswap feature in a
> separate file to not get noise from the full bswap. However I still
> don't like the idea of notifying for partial bswap in statements that
> will eventually be eliminated as dead statements. Besides, this
> occur because some recursion is made for each statement even
> if they were already examined which is not very nice in itself.

Ah, indeed ...

> The solution I see is to keep a map of visited statements but I
> don't know if that's fine in this case. After all, it's not strictly
> necessary as the pass would work without this. It would just serve
> to avoid redundant computation and confusion statistics.

... having bswap cleanup after itself (thus remove dead code itself
would be nice).  Let's just keep the above as a possibility for
further enhancements and focus on getting the current patch
correct and committed.

Btw, another enhancement is memory target.  Thus recognize

void bswap_mem (char *m1, char *m2)
{
  m2[0] = m1[3];
  m2[1] = m1[2];
  m2[2] = m1[1];
  m2[3] = m1[0];
}

with all the complication that arises due to aliasing issues.

Thanks,
Richard.

> Best regards,
>
> Thomas
>
>


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