Spurious data dependency in aes64ks1i instruction #3255
Comments
ZenithalHourlyRate
added a commit
that referenced
this issue
Feb 16, 2023
It is not rs2, it is imm Related to #3255
|
#3256 may fix the issue. |
ZenithalHourlyRate
added a commit
that referenced
this issue
Feb 16, 2023
It is not rs2, it is imm Related to #3255
sequencer
pushed a commit
that referenced
this issue
Feb 21, 2023
It is not rs2, it is imm Related to #3255
sequencer
pushed a commit
that referenced
this issue
Feb 21, 2023
It is not rs2, it is imm Related to #3255
sequencer
pushed a commit
that referenced
this issue
Feb 22, 2023
It is not rs2, it is imm Related to #3255
|
Should be resolved now. |
jerryz123
pushed a commit
that referenced
this issue
Feb 22, 2023
It is not rs2, it is imm Related to #3255
jerryz123
pushed a commit
that referenced
this issue
Feb 22, 2023
It is not rs2, it is imm Related to #3255
sequencer
pushed a commit
that referenced
this issue
Feb 23, 2023
It is not rs2, it is imm Related to #3255
sequencer
added a commit
that referenced
this issue
Aug 22, 2023
* bump to Chisel 3.5.6 (#3222) * Remove deprecated code for BarrelShifter Cherry-picked chipsalliance/chisel@7372c9e Should use BarrelShifter from chisel3.std, but it is not published, see chipsalliance/chisel#2997 * Fix scala reflect error scala.reflect.internal.Types: constructor RecordMap in class RecordMap cannot be accessed in package util from package util in package rocketchip * explicit add legacy connect operators * replace all cde dependencies. * change api-config to cde in build.sbt * bump cde submodule * fix Makefile * IDecode: Fix aes64ks1i imm decode It is not rs2, it is imm Related to #3255 * CryptoNIST: refactor rnum This removes a redundant port As rnum is encoded in rs2 as imm now, we can get it just from rs2 * Remove redundant TLBExceptions V bit This was introduced by "(185cac8) Add hypervisor extension (#2841)" This is a dead code, as no circuit is producing and consuming this bit. This was discovered when migrating Core.scala to chisel3, where strict checking was applied for IO and firrtl found this is not connected. In the context of hypervisor extension, V bit, or _virtualization mode_, indicates whether the hart is currently executing in a guest. For TLBReq, the V bit is needed as it affects the PTW thus TLB behavior on whether to do _Two-Stage Address Translation_. However, the bit is not needed for TLBException. The exceptions (pf, gf, ae, ma) have no V=1/V=0 variants. Also, there is no point to add V bit for gf (guest page fault). The io.resp.gf added below in the original patch also does not connect io.resp.gf.v. If this V bit should be added for this specific exception, it should be connected. I assume this was added accidentally, as the original "extends CoreBundle()(p)" modification seems irrelevant. * update build system for cde bump * Fix unconnected io.start for TLRAMXbarTest and TLMulticlientXbarTest (cherry picked from commit 34d7309) * Fold HasPeripheryDebugModuleImp into HasPeripheryDebug (cherry picked from commit a2682ca) * Move HasDebugModule out of TileContextType (cherry picked from commit 0e4af6d) * Pinning nix to 2.13.3 in github workflows Nix 2.14 released with incompatibilities with cachix/install-nix-action. This PR pins nix to 2.13.3 to avoid CI fails See: cachix/install-nix-action#161 (cherry picked from commit 84533ae) * L1TLB: VS-mode SFENCE misses tera/giga-page entries fragmented superpages (#3297) (cherry picked from commit 8b52a6f) * feat: port Chisel2 to Chisel3 devices/ (cherry picked from commit 1ff0db3) * fix: add `chiselTypeOf` when inst Wire(in.d) (cherry picked from commit e645d94) * feat: port Chisel2 to Chisel3 rocket/ (cherry picked from commit d6a982b) * feat: port Chisel2 to Chisel3 amba/ (cherry picked from commit b2fd991) * Fix CharCount RoCC example bug Reset recv_beat (cherry picked from commit f19a90a) * Change CharCountRoCC Example to use dcacheParams RoCC accesses D$, not I$ (cherry picked from commit 61ea81c) * mill: fix empty cross arg for riscv-tests.Suite riscv-tests.suite[] wont compile (cherry picked from commit d86c011) * Hypervisor: encodeVirtualAddress extra MSB to canonicalize VS-disabled (#3314) (cherry picked from commit 0504a9b) * Support devOverride for diplomatic SRAMs (cherry picked from commit 12e21a6) * Support overriding the DTS node for diplomatic SRAMs (cherry picked from commit c8edec3) * Fix no-debug-node designs (cherry picked from commit 3b8d3c1) * all isaDTS strings to lowercase (#3333) (#3334) (cherry picked from commit 58c8249) Co-authored-by: Yangyu Chen <cyy@cyyself.name> * Deprecate old BusWrapper methods (#3337) (#3340) (cherry picked from commit 2570db7) Co-authored-by: Jerry Zhao <jerryz123@berkeley.edu> * Fix AXI4 RegisterRouter on Wire Clone (#3341) In legacy code it is ok to clone a Wire using Wire() now the requirement is more strict and WireDefault is needed Related to #3059 The Output is added for code style consistency Closes #3324 (cherry picked from commit fb5d7d0) Co-authored-by: Zenithal <i@zenithal.me> * Support TLFilter.mSubtract with multiple AddressSets (#3339) (#3343) Enables filtering out multiple ranges at once. (cherry picked from commit 4110563) Co-authored-by: Jerry Zhao <jerryz123@berkeley.edu> * Improve boundaryBuffers API | move boundaryBuffers to within-Tile (#3342) (#3344) * Add API to force TLBuffers into RocketTile as boundaryBuffers * Generate boundaryBuffers within the Tile, not the TilePRCIDomain (cherry picked from commit 3f74d79) Co-authored-by: Jerry Zhao <jerryz123@berkeley.edu> * Implement Zicond extension (#3329) * Instructions: sync from riscv-opcodes (add zicond) * Zicond: implement czero.eqz, czero.nez * Zicond: fix implementation * Zicond: reduce code duplication * Add ISA extension when using Zicond * Fix ISA extensions ordering (cherry picked from commit 62162c5) * feat: port Chisel2 to Chisel3 tilelink/ (cherry picked from commit 6c23100) * fix: not fully initialized wires in Edge.scala + using `DontCare` (cherry picked from commit 3902497) * fix: add chiselTypeOf when inst Wire + in.d and out.a; (cherry picked from commit e52773e) * fix: not fully initialized wires in Edge.scala + using `DontCare` (cherry picked from commit 5f7278b) * Revert to old := connects instead of :#= * Support removing the nonstandard CEASE from rocket (cherry picked from commit 956a1ff) * fix: Chisel3 #2944 Move SourceInfo to package experimental * Update debug_rom.S (cherry picked from commit a9ae0b9) * Update debug_rom_nonzero.S (cherry picked from commit 6ba9437) * Remove cloneType Ref to #2889 * Bump hardfloat * Bump to chisel3.6 * Switch to json4s-native * Update HeterogeneousBag to chisel3.6 * Bump mill flow to chisel 3.6.0 * Switch HeterogeneousBag to VectorMap * Update src/main/scala/util/HeterogeneousBag.scala Co-authored-by: Jack Koenig <koenig@sifive.com> * support circt and bump to 3.6 * Generalize Vec[TracedInstruction] to a TraceBundle (cherry picked from commit 85aca71) * Add time to TraceBundle (cherry picked from commit 57af718) * Supporting adding custom stuff to TraceBundle (cherry picked from commit f6f59c1) * Fix BlockableTraceBundle (cherry picked from commit efa8337) * Bump nix toolchain version * Fix TLSourceShrinker (cherry picked from commit d503368) * Support RoCC accels which define CSRs (#3358) (cherry picked from commit 7ddf02a) * Add support for an unsynthesizable ROB to produce a TracedInstruction stream from Rocket with wdata (cherry picked from commit d6c09c9) # Conflicts: # src/main/scala/rocket/RocketCore.scala # src/main/scala/subsystem/Configs.scala * Fix memory leak in debug_rob (cherry picked from commit b3f391c) * Fix unittests (cherry picked from commit a806851) * Fix TL unittests (cherry picked from commit ecf08f5) * Support blockable credited interfaces (cherry picked from commit c8cf935) * Make AsyncQueue use Rawmodule (cherry picked from commit 8db7364) * Support dynamic credit count in senders for CreditedIO (cherry picked from commit 5a5c127) * Fix TLJbarTest (cherry picked from commit e76a4ea) * Add [m/s/h]envcfg CSRs to satisfy spec requirements (#3373) (cherry picked from commit 9967142) * fix: Werror match may not be exhaustive (#3268) (#3384) + add `case _` to make match be exhaustive (cherry picked from commit 05d9db7) Co-authored-by: SingularityKChen <chency_singularity@163.com> * fix: empty argument list (#3262) (#3386) + add `()` to those empty argument list function call; Auto-application to `()` is deprecated. Supply the empty argument list `()` explicitly to invoke method (cherry picked from commit 9b383c5) Co-authored-by: SingularityKChen <chency_singularity@163.com> * fix: bit extraction use .U.extract(i) (#3263) (#3385) + replace `.U(i)` with `.U.extract(i)` (cherry picked from commit 9a1dc2d) Co-authored-by: SingularityKChen <chency_singularity@163.com> * TLB: must_alloc swapped AMO Logical/Arithmetic (#3390) (cherry picked from commit dc275c4) Co-authored-by: John Ingalls <43973001+ingallsj@users.noreply.github.com> * TLB: vsstatus.SUM check should not apply to Stage-2 fault before Stage-1 response (#3393) (#3399) (cherry picked from commit 43e0af1) Co-authored-by: John Ingalls <43973001+ingallsj@users.noreply.github.com> * CSR: optionally set delegable hypervisor exceptions (#3401) Given that usingHypervisor is used to distinguish whether the hardware supports hypervisor extensions, we should use it for the delegable exceptions as well. (cherry picked from commit 026f4c9) * Enable WARL custom CSRs, long-latency CSR accesses (#3388) * Support setting custom CSRs from datapath * Support CSR stalls (cherry picked from commit 005c6db) * PTW: set ae_ptw for out-of-range non-leaf PTEs (#3407) (#3410) For PTEs whose physical address is out-of-range, we need to set `ae_ptw` instead of `ae_final` to raise access-fault. Because non-leaf PTEs will not have R or X bits set, `ae_final` will be overrided by page-fault exceptions. (cherry picked from commit b8dad7f) Co-authored-by: Yinan Xu <xuyinan@ict.ac.cn> --------- Co-authored-by: Jiuyang Liu <liu@jiuyang.me> Co-authored-by: Zenithal <i@zenithal.me> Co-authored-by: Hansung Kim <hansung_kim@berkeley.edu> Co-authored-by: Liu Xiaoyi <circuitcoder0@gmail.com> Co-authored-by: John Ingalls <43973001+ingallsj@users.noreply.github.com> Co-authored-by: singularity <chency_singularity@163.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Yangyu Chen <cyy@cyyself.name> Co-authored-by: rewired <39949564+rewired-gh@users.noreply.github.com> Co-authored-by: Jack Koenig <koenig@sifive.com> Co-authored-by: Yinan Xu <xuyinan@ict.ac.cn>
jerryz123
added a commit
that referenced
this issue
Nov 4, 2023
* bump to Chisel 3.5.6 (#3222) * Remove deprecated code for BarrelShifter Cherry-picked chipsalliance/chisel@7372c9e Should use BarrelShifter from chisel3.std, but it is not published, see chipsalliance/chisel#2997 * Fix scala reflect error scala.reflect.internal.Types: constructor RecordMap in class RecordMap cannot be accessed in package util from package util in package rocketchip * explicit add legacy connect operators * replace all cde dependencies. * change api-config to cde in build.sbt * bump cde submodule * fix Makefile * IDecode: Fix aes64ks1i imm decode It is not rs2, it is imm Related to #3255 * CryptoNIST: refactor rnum This removes a redundant port As rnum is encoded in rs2 as imm now, we can get it just from rs2 * Remove redundant TLBExceptions V bit This was introduced by "(185cac8) Add hypervisor extension (#2841)" This is a dead code, as no circuit is producing and consuming this bit. This was discovered when migrating Core.scala to chisel3, where strict checking was applied for IO and firrtl found this is not connected. In the context of hypervisor extension, V bit, or _virtualization mode_, indicates whether the hart is currently executing in a guest. For TLBReq, the V bit is needed as it affects the PTW thus TLB behavior on whether to do _Two-Stage Address Translation_. However, the bit is not needed for TLBException. The exceptions (pf, gf, ae, ma) have no V=1/V=0 variants. Also, there is no point to add V bit for gf (guest page fault). The io.resp.gf added below in the original patch also does not connect io.resp.gf.v. If this V bit should be added for this specific exception, it should be connected. I assume this was added accidentally, as the original "extends CoreBundle()(p)" modification seems irrelevant. * update build system for cde bump * Fix unconnected io.start for TLRAMXbarTest and TLMulticlientXbarTest (cherry picked from commit 34d7309) * Fold HasPeripheryDebugModuleImp into HasPeripheryDebug (cherry picked from commit a2682ca) * Move HasDebugModule out of TileContextType (cherry picked from commit 0e4af6d) * Pinning nix to 2.13.3 in github workflows Nix 2.14 released with incompatibilities with cachix/install-nix-action. This PR pins nix to 2.13.3 to avoid CI fails See: cachix/install-nix-action#161 (cherry picked from commit 84533ae) * L1TLB: VS-mode SFENCE misses tera/giga-page entries fragmented superpages (#3297) (cherry picked from commit 8b52a6f) * feat: port Chisel2 to Chisel3 devices/ (cherry picked from commit 1ff0db3) * fix: add `chiselTypeOf` when inst Wire(in.d) (cherry picked from commit e645d94) * feat: port Chisel2 to Chisel3 rocket/ (cherry picked from commit d6a982b) * feat: port Chisel2 to Chisel3 amba/ (cherry picked from commit b2fd991) * Fix CharCount RoCC example bug Reset recv_beat (cherry picked from commit f19a90a) * Change CharCountRoCC Example to use dcacheParams RoCC accesses D$, not I$ (cherry picked from commit 61ea81c) * mill: fix empty cross arg for riscv-tests.Suite riscv-tests.suite[] wont compile (cherry picked from commit d86c011) * Hypervisor: encodeVirtualAddress extra MSB to canonicalize VS-disabled (#3314) (cherry picked from commit 0504a9b) * Support devOverride for diplomatic SRAMs (cherry picked from commit 12e21a6) * Support overriding the DTS node for diplomatic SRAMs (cherry picked from commit c8edec3) * Fix no-debug-node designs (cherry picked from commit 3b8d3c1) * all isaDTS strings to lowercase (#3333) (#3334) (cherry picked from commit 58c8249) Co-authored-by: Yangyu Chen <cyy@cyyself.name> * Deprecate old BusWrapper methods (#3337) (#3340) (cherry picked from commit 2570db7) Co-authored-by: Jerry Zhao <jerryz123@berkeley.edu> * Fix AXI4 RegisterRouter on Wire Clone (#3341) In legacy code it is ok to clone a Wire using Wire() now the requirement is more strict and WireDefault is needed Related to #3059 The Output is added for code style consistency Closes #3324 (cherry picked from commit fb5d7d0) Co-authored-by: Zenithal <i@zenithal.me> * Support TLFilter.mSubtract with multiple AddressSets (#3339) (#3343) Enables filtering out multiple ranges at once. (cherry picked from commit 4110563) Co-authored-by: Jerry Zhao <jerryz123@berkeley.edu> * Improve boundaryBuffers API | move boundaryBuffers to within-Tile (#3342) (#3344) * Add API to force TLBuffers into RocketTile as boundaryBuffers * Generate boundaryBuffers within the Tile, not the TilePRCIDomain (cherry picked from commit 3f74d79) Co-authored-by: Jerry Zhao <jerryz123@berkeley.edu> * Implement Zicond extension (#3329) * Instructions: sync from riscv-opcodes (add zicond) * Zicond: implement czero.eqz, czero.nez * Zicond: fix implementation * Zicond: reduce code duplication * Add ISA extension when using Zicond * Fix ISA extensions ordering (cherry picked from commit 62162c5) * feat: port Chisel2 to Chisel3 tilelink/ (cherry picked from commit 6c23100) * fix: not fully initialized wires in Edge.scala + using `DontCare` (cherry picked from commit 3902497) * fix: add chiselTypeOf when inst Wire + in.d and out.a; (cherry picked from commit e52773e) * fix: not fully initialized wires in Edge.scala + using `DontCare` (cherry picked from commit 5f7278b) * Revert to old := connects instead of :#= * Support removing the nonstandard CEASE from rocket (cherry picked from commit 956a1ff) * fix: Chisel3 #2944 Move SourceInfo to package experimental * Update debug_rom.S (cherry picked from commit a9ae0b9) * Update debug_rom_nonzero.S (cherry picked from commit 6ba9437) * Remove cloneType Ref to #2889 * Bump hardfloat * Bump to chisel3.6 * Switch to json4s-native * Update HeterogeneousBag to chisel3.6 * Bump mill flow to chisel 3.6.0 * Switch HeterogeneousBag to VectorMap * Update src/main/scala/util/HeterogeneousBag.scala Co-authored-by: Jack Koenig <koenig@sifive.com> * support circt and bump to 3.6 * Generalize Vec[TracedInstruction] to a TraceBundle (cherry picked from commit 85aca71) * Add time to TraceBundle (cherry picked from commit 57af718) * Supporting adding custom stuff to TraceBundle (cherry picked from commit f6f59c1) * Fix BlockableTraceBundle (cherry picked from commit efa8337) * Bump nix toolchain version * Fix TLSourceShrinker (cherry picked from commit d503368) * Support RoCC accels which define CSRs (#3358) (cherry picked from commit 7ddf02a) * Add support for an unsynthesizable ROB to produce a TracedInstruction stream from Rocket with wdata (cherry picked from commit d6c09c9) # Conflicts: # src/main/scala/rocket/RocketCore.scala # src/main/scala/subsystem/Configs.scala * Fix memory leak in debug_rob (cherry picked from commit b3f391c) * Fix unittests (cherry picked from commit a806851) * Fix TL unittests (cherry picked from commit ecf08f5) * Support blockable credited interfaces (cherry picked from commit c8cf935) * Make AsyncQueue use Rawmodule (cherry picked from commit 8db7364) * Support dynamic credit count in senders for CreditedIO (cherry picked from commit 5a5c127) * Fix TLJbarTest (cherry picked from commit e76a4ea) * Add [m/s/h]envcfg CSRs to satisfy spec requirements (#3373) (cherry picked from commit 9967142) * fix: Werror match may not be exhaustive (#3268) (#3384) + add `case _` to make match be exhaustive (cherry picked from commit 05d9db7) Co-authored-by: SingularityKChen <chency_singularity@163.com> * fix: empty argument list (#3262) (#3386) + add `()` to those empty argument list function call; Auto-application to `()` is deprecated. Supply the empty argument list `()` explicitly to invoke method (cherry picked from commit 9b383c5) Co-authored-by: SingularityKChen <chency_singularity@163.com> * fix: bit extraction use .U.extract(i) (#3263) (#3385) + replace `.U(i)` with `.U.extract(i)` (cherry picked from commit 9a1dc2d) Co-authored-by: SingularityKChen <chency_singularity@163.com> * TLB: must_alloc swapped AMO Logical/Arithmetic (#3390) (cherry picked from commit dc275c4) Co-authored-by: John Ingalls <43973001+ingallsj@users.noreply.github.com> * TLB: vsstatus.SUM check should not apply to Stage-2 fault before Stage-1 response (#3393) (#3399) (cherry picked from commit 43e0af1) Co-authored-by: John Ingalls <43973001+ingallsj@users.noreply.github.com> * CSR: optionally set delegable hypervisor exceptions (#3401) Given that usingHypervisor is used to distinguish whether the hardware supports hypervisor extensions, we should use it for the delegable exceptions as well. (cherry picked from commit 026f4c9) * Enable WARL custom CSRs, long-latency CSR accesses (#3388) * Support setting custom CSRs from datapath * Support CSR stalls (cherry picked from commit 005c6db) * PTW: set ae_ptw for out-of-range non-leaf PTEs (#3407) (#3410) For PTEs whose physical address is out-of-range, we need to set `ae_ptw` instead of `ae_final` to raise access-fault. Because non-leaf PTEs will not have R or X bits set, `ae_final` will be overrided by page-fault exceptions. (cherry picked from commit b8dad7f) Co-authored-by: Yinan Xu <xuyinan@ict.ac.cn> --------- Co-authored-by: Jiuyang Liu <liu@jiuyang.me> Co-authored-by: Zenithal <i@zenithal.me> Co-authored-by: Hansung Kim <hansung_kim@berkeley.edu> Co-authored-by: Liu Xiaoyi <circuitcoder0@gmail.com> Co-authored-by: John Ingalls <43973001+ingallsj@users.noreply.github.com> Co-authored-by: singularity <chency_singularity@163.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Yangyu Chen <cyy@cyyself.name> Co-authored-by: rewired <39949564+rewired-gh@users.noreply.github.com> Co-authored-by: Jack Koenig <koenig@sifive.com> Co-authored-by: Yinan Xu <xuyinan@ict.ac.cn>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Type of issue: bug report
Impact: performance impact
Other information
During formal verification with the BitManipCryptoConfig, we see the instruction in question being needlessly replayed. Consider the code sequence
where the initial load has a cache miss. This is causing the aes64ks1i to replay without any actual dependency on the load. Apparently, the core thinks that aes64ks1i is using the rs2 field (encoding is actually 00 11000 1 rnum[3:0] rs1 001 rd 0010011), meaning a spurious dependency on x19 in this case with rnum=3.
I assume this is also causing needless stalls in decode.
This has been identified by running the Siemens Processor Verification App on the generated RTL.
The text was updated successfully, but these errors were encountered: