RFR: Store verification
Roman Kennke
rkennke at redhat.com
Thu Oct 20 16:31:07 UTC 2016
Am Donnerstag, den 20.10.2016, 16:46 +0100 schrieb Andrew Haley:
> On 20/10/16 14:48, Roman Kennke wrote:
> >
> > http://cr.openjdk.java.net/~rkennke/storechecks/webrev/
> >
> > Ok to push?
>
> I really don't like all those boolean constants "true" and "false"
> all
> over the place.
Me neither.
> Yeah, I know I've done it elsewhere in aarch64.ad and
> it's bad style. Mea culpa, but I don't want it to spread any
> further.
>
> Please either define loadInsn() and storeInsn()
We've got 3 loadStore() methods, and one MOV_VOLATILE macro, and they
are multiline, 2 of them >10lines and duplicating those, with only 1
line difference seems ugly...
> or define an enum
>
> enum (isLoad, isStore)
>
> and use it instead of a bool.
Sounds more useful. Will do that then.
> + orr(rval, zr, value);
> + // mov(rval, value);
> + mov(raddr, addr);
>
> What is this change for? Why orr instead of mov?
A leftover from some debugging. Will turn in back into mov. Good find!
> + // Macro instructions for accessing and updating the condition
> flags
> + inline void get_cflags(Register reg)
> + {
> + mrs(0b011, 0b0100, 0b0010, 0b000, reg);
> + }
> +
> + inline void set_cflags(Register reg)
> + {
> + msr(0b011, 0b0100, 0b0010, 0b000, reg);
> + }
> +
>
> get_nzcv() would be easier for the reader to understand, and matches
> ARM's assembly language.
Ok. Will post another webrev soon.
Roman
More information about the shenandoah-dev
mailing list