RE: [PATCH] Hexagon: add PC alignment check and exception

[Matheus Tavares Bernardino <quic_mathbern@quicinc.com>]

Hi, Taylor

On Mon, 29 Apr 2024 09:51:16 -0500 <ltaylorsimpson@gmail.com> wrote:
>
> PS  You should also update the pkt_raises_exception function in translate.c
> to return true for packets that contain these instructions.  This will
> ensure that none of the machine state is changed before the check is
> complete.
 
Will do, thanks.

> > -----Original Message-----
> > From: ltaylorsimpson@gmail.com <ltaylorsimpson@gmail.com>
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> > >
> > >
> > > --- a/target/hexagon/genptr.c
> > > +++ b/target/hexagon/genptr.c
> > > @@ -473,6 +473,7 @@ static void gen_write_new_pc_addr(DisasContext
> > 
> > You haven't added the check to gen_write_new_pc_pcrel.  It's not needed
> > there because the encoding guarantees the target is always aligned -
> right?
> > However, there is a call to gen_write_new_pc_addr inside that function.
> In
> > this case, we'll add a check that isn't necessary.  Consider adding a
> parameter
> > to indicate if the check can be avoided.

Actually, I had missed this spot and I think we do need the check at 
gen_write_new_pc_pcrel too. The added test `unaligned_pc_multi_cof.S` will
exercise it with a relative PC addr that is not aligned. I'll fix that, thanks. 

> > > a/tests/tcg/hexagon/Makefile.target
> > > b/tests/tcg/hexagon/Makefile.target
> > > index f839b2c0d5..02d7fff34c 100644
> > > --- a/tests/tcg/hexagon/Makefile.target
> > > +++ b/tests/tcg/hexagon/Makefile.target
> > > @@ -51,6 +51,19 @@ HEX_TESTS += scatter_gather  HEX_TESTS +> hvx_misc
> > > HEX_TESTS += hvx_histogram  HEX_TESTS += invalid-slots
> > > +HEX_TESTS += unaligned_pc
> > > +HEX_TESTS += unaligned_pc_multi_cof
> > > +
> > > +run-unaligned_pc: unaligned_pc
> > > +run-unaligned_pc_multi_cof: unaligned_pc_multi_cof run-unaligned_pc
> > > +run-unaligned_pc_multi_cof:
> > > +	$(call run-test, $<, $(QEMU) $< 2> $<.stderr,"$< on
> > > $(TARGET_NAME)"); \
> > > +	if [ $$? -ne 1 ] ; then \
> > > +		return 1; \
> > > +	fi
> > > +	$(call quiet-command, \
> > > +		grep -q "exception 0x1e" $<.stderr, \
> > > +		"GREP", "exception 0x1e");
> > 
> > We should also test endloop instructions.

Good idea, will do.