[PATCH] D39053: [Bitfield] Add more cases to making the bitfield a separate location

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D39053#906513, @spetrovic wrote:

> Well, basically I'm just expanding the existing algorithm, why should we split fields just in case when current field is integer,
>  I'm not resolving specific problem with unaligned loads/stores on MIPS.
>
> In this example:
>
> typedef struct {
>
>   unsigned int f1 : 28;
>   unsigned int f2 : 4;
>   unsigned int f3 : 12;
>
> } S5;
>
> S5 *cmd;
>
> void foo() {
>
>   	cmd->f3 = 5;
>
> }
>
> With this patch there is improvement in code size not just on MIPS architecture, on X86 and ARM is also improved code size. If structure S5 is treated as i48 type there are extra instructions for reading it not just on MIPS. We can take results for MIPS just for example:
>
> Output without the patch:
>
> 0000000000000000 <foo>:
>
>    0:	3c010000 	lui	at,0x0
>    4:	0039082d 	daddu	at,at,t9
>    8:	64210000 	daddiu	at,at,0
>    c:	dc210000 	ld	at,0(at)
>   10:	dc210000 	ld	at,0(at)
>   14:	68220000 	ldl	v0,0(at)
>   18:	6c220007 	ldr	v0,7(at)
>   1c:	64030005 	daddiu	v1,zero,5
>   20:	7c62fd07 	dins	v0,v1,0x14,0xc
>   24:	b0220000 	sdl	v0,0(at)
>   28:	03e00008 	jr	ra
>   2c:	b4220007 	sdr	v0,7(at)
>   
>   
>
> Output with the patch:
>
> 0000000000000000 <foo>:
>
>    0:	3c010000 	lui	at,0x0
>    4:	0039082d 	daddu	at,at,t9
>    8:	64210000 	daddiu	at,at,0
>    c:	dc210000 	ld	at,0(at)
>   10:	dc210000 	ld	at,0(at)
>   14:	94220004 	lhu	v0,4(at)
>   18:	24030005 	li	v1,5
>   1c:	7c62f904 	ins	v0,v1,0x4,0x1c
>   20:	03e00008 	jr	ra
>   24:	a4220004 	sh	v0,4(at)
>   
>
> This is simple example, in more complicated examples there is more improvement.


I think this is part of the slippery slope we didn't want to go down. We introduced this mode in the first place only to resolve a store-to-load forwarding problem that is theoretically unsolvable by any local lowering decisions. This, on the other hand, looks like a local code-generation problem that we can/should fix in the backend. If I'm wrong, we should consider this as well.


https://reviews.llvm.org/D39053