Zmmul: Multiply without Divide Extension

[nananapo]

Type of change: other enhancement

Impact: API addition (no impact on existing code)

Development Phase: implementation

Release Notes

This PR is to add Zmmul extension on the rocket-chip.
This adds a new Config WithoutDiv and a new parameter divEnabled in MulDivParams.
When divEnabled is false, div logic of MulDiv module is disabled. And div/rem instruction is not included in a decode table.

[ZenithalHourlyRate]

Overall good to me! Though the software design for enabling Zmmul needs further discussion, including the config name. WithMulDivWithoutDiv seems confusing.

Could you demo with an emulator that a div instruction will cause illegal instruction exception in this config?

Could you add an arch-test CI for zmmul?

[ZenithalHourlyRate]

context: We had some prior private communication.

After seeing the code things become more clear to me, It seems that useMulDivWithoutDiv can be merged into MulDiv by setting divUnroll = 0 instead of a standalone option?

I favor this simple way. And for the exposed interface to the end user, he/she only needs to use some config like WithoutDiv and that config will set divUnroll = 0

But this is tricky as divUnroll == 0 has two purpose for developer now

1. Zmmul, namely turning off div.
2. Alternative divider impl suggested in Teach MulDiv to do div-only #1228

[ZenithalHourlyRate]

After discussion with Jerry, it is concluded that adding another option inside MulDivParams like divUnroll: Option[Int] or (divUnroll: Int and divEnable: Boolean = true) should be used, instead of another boolean in CoreParams. The later way is prefered as it wont break compatibility.

[ZenithalHourlyRate]

1. zmmul should be placed after zihpm instead of appending to the end

2. Some interesting thing: rv32imac_zmmul is a valid ISA string.

[ZenithalHourlyRate]

Should not set mulUnroll here.

[ZenithalHourlyRate]

Other parts look good to me!

[ZenithalHourlyRate]

This should default to true.

[ZenithalHourlyRate]

Forgot to start a review...

Actually it should be

      Option.when(tileParams.core.mulDiv.nonEmpty)(Seq("zmmul")) ++

Check riscv/riscv-isa-manual#869 for more details.

And apply the following diff to the m instead

-val m = if (tileParams.core.mulDiv.nonEmpty) "m" else ""
+val m = if (tileParams.core.mulDiv.nonEmpty  && !tileParams.core.mulDiv.get.divEnabled) "m" else ""

Also you should update the MISA generation in CSR.scala

[nananapo]

Hi.
If I understand correctly, there is a diferrence between rv32im_zmmul and rv32im about mul inst when misa.M=0.
If I change ISA of rocket-chip to rv32im_zmmul when mulDiv.nonEmpty, the behaviour of mul inst with misa.M=0 will be changed to not cause illegal instruction exception.
Is it OK?

[ZenithalHourlyRate]

If I understand correctly, there is a diferrence between rv32im_zmmul and rv32im about mul inst when misa.M=0.

It does exist in that thread, but from my perception it was not the focus of that thread and it was not finally decided.

From riscv/riscv-isa-manual#869 (comment), a future Sm* extension should come to solve the problem. As it does not exist for now, I assume this detail is undefined, so it is now implementation-defined.

If I change ISA of rocket-chip to rv32im_zmmul when mulDiv.nonEmpty, the behaviour of mul inst with misa.M=0 will be changed to not cause illegal instruction exception.

I think we should not change the behavior, as it is undefined, and counter-intuitive.

I think we should officially support only two modes: m and zmmul. As for m_zmmul, it is equivalent to m and the misa behavior is the same as m.

Now I wont insist that we should output m_zmmul for m. I am OK if you want to just output m or zmmul in the ISA string. One final commit/comment and I will merge.