On Sat, Jun 1, 2019 at 5:53 PM Martin Sebor <mse...@gmail.com> wrote: > > I spent a bunch of time the other day trying to understand why > the second of the two assignments below to a char array was > apparently not being done by trunk > > a[0] = 1; > a[1] = 0; > > The optimized GIMPLE dump simply shows: > > MEM[(char *)&a] = 1; > > when in the past it showed: > > MEM[(char[2] *)&a2] = 1; > > After some debugging I figured out that this is the result of > the store merging pass transforming the two assignments into > one: > > *(short int *)a = 1; > > and the MEM_REF dump mentioning only the type of the second > operand and not the type of the access. > > To avoid this confusion the attached patch adds to the dump > a cast to the MEM_REF type for accesses whose size is not equal > to the size of the operand (when the sizes are the same no new > cast is prepended). The effect is that with store merging in > effect, the dump for the above becomes > > MEM[(short int *)(char *)&a] = 1;
I think this is confusing syntax. Iff you absolutely refuse to make the -gimple dump the default for MEM_REF and you insist on fixing this issue then please follow how we dump VIEW_CONVERT_EXPR which is the only other tree code we dump the access type, thus MEM<short int *>[(char *)&a] = 1; btw, -gimple (iff only applied to MEM_REF) would have dumped __MEM <short int, 8> ((char *)&a2) = 1; also including the effective alignment of the access. Your variant suggests that the alignment is that of short int which is wrong. Yes, there's testsuite fallout I guess mostly about () vs. [] and spacing (the GIMPLE FE doesn't insist on spacing before/after <>). So please do not change MEM_REF dumping in this way. Richard. > This should make both the size and the type of the access clear > and help avoid the confusion. The output isn't the same as in > earlier releases because because the access really is done via > a short pointer and not as an array of char. > > There is more detail in MEM_REF that could be included here but > it seems that the size of the access is essential to interpreting > the dumps. > > Tested on x86_64-linux with only minimal testsuite fallout. > > Martin
