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