[riscv-port] RFR: 8278041: riscv: Use t2 as the dedicated machine flags register in C2
There are non-allocatable registers t0 and t1 in C2, frequently used as temporary registers for common code snipptes. Meanwhile, C2 use t1 as the flags register, so that many instructs using t1 as a temporary register must declare the data flow effect of t1 which is not part of a match rule. So we can reserve t2 and use it as the machine flags register in C2 instead, to reduce the probability of misuse of t1 without declaring the data flow effect. ------------- Commit messages: - 8278041: riscv: Use t2 as the dedicated machine flags register in C2 Changes: https://git.openjdk.java.net/riscv-port/pull/21/files Webrev: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=21&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278041 Stats: 144 lines in 4 files changed: 3 ins; 24 del; 117 mod Patch: https://git.openjdk.java.net/riscv-port/pull/21.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/21/head:pull/21 PR: https://git.openjdk.java.net/riscv-port/pull/21
On Wed, 1 Dec 2021 07:56:54 GMT, Yadong Wang <yadongwang@openjdk.org> wrote:
There are non-allocatable registers t0 and t1 in C2, frequently used as temporary registers for common code snipptes. Meanwhile, C2 use t1 as the flags register, so that many instructs using t1 as a temporary register must declare the data flow effect of t1 which is not part of a match rule. So we can reserve t2 and use it as the machine flags register in C2 instead, to reduce the probability of misuse of t1 without declaring the data flow effect.
This will surely make the code more maintainable. Nice cleanup :-) No obvious performance impact is witnessed by running specjbb2015 with 't2' used as the dedicate C2 machine flags register. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/21
On Wed, 1 Dec 2021 07:56:54 GMT, Yadong Wang <yadongwang@openjdk.org> wrote:
There are non-allocatable registers t0 and t1 in C2, frequently used as temporary registers for common code snipptes. Meanwhile, C2 use t1 as the flags register, so that many instructs using t1 as a temporary register must declare the data flow effect of t1 which is not part of a match rule. So we can reserve t2 and use it as the machine flags register in C2 instead, to reduce the probability of misuse of t1 without declaring the data flow effect.
Looks like we missed instructs like: decodeKlass_not_null_with_tmp, string_indexofUU, etc. ? Maybe you can search "rFlagsReg tmp" to find. ------------- Changes requested by fyang (Lead). PR: https://git.openjdk.java.net/riscv-port/pull/21
There are non-allocatable registers t0 and t1 in C2, frequently used as temporary registers for common code snipptes. Meanwhile, C2 use t1 as the flags register, so that many instructs using t1 as a temporary register must declare the data flow effect of t1 which is not part of a match rule. So we can reserve t2 and use it as the machine flags register in C2 instead, to reduce the probability of misuse of t1 without declaring the data flow effect.
Yadong Wang has updated the pull request incrementally with one additional commit since the last revision: 8278041: riscv: Use t2 as the dedicated machine flags register in C2(2) ------------- Changes: - all: https://git.openjdk.java.net/riscv-port/pull/21/files - new: https://git.openjdk.java.net/riscv-port/pull/21/files/b073e926..2e01c8a4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=21&range=01 - incr: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=21&range=00-01 Stats: 48 lines in 1 file changed: 0 ins; 31 del; 17 mod Patch: https://git.openjdk.java.net/riscv-port/pull/21.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/21/head:pull/21 PR: https://git.openjdk.java.net/riscv-port/pull/21
On Wed, 1 Dec 2021 09:35:26 GMT, Yadong Wang <yadongwang@openjdk.org> wrote:
There are non-allocatable registers t0 and t1 in C2, frequently used as temporary registers for common code snipptes. Meanwhile, C2 use t1 as the flags register, so that many instructs using t1 as a temporary register must declare the data flow effect of t1 which is not part of a match rule. So we can reserve t2 and use it as the machine flags register in C2 instead, to reduce the probability of misuse of t1 without declaring the data flow effect.
Yadong Wang has updated the pull request incrementally with one additional commit since the last revision:
8278041: riscv: Use t2 as the dedicated machine flags register in C2(2)
Hi Yadong, it looks cost too much to reserve a dedicated register for control flag. I think in most cases we can use the 2 scratch registers instead. Can you show some complicated cases? We can check if they can be rewrite to avoid the complication. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/21
On Wed, 1 Dec 2021 13:11:44 GMT, kuaiwei <duke@openjdk.java.net> wrote:
Hi Yadong, it looks cost too much to reserve a dedicated register for control flag. I think in most cases we can use the 2 scratch registers instead. Can you show some complicated cases? We can check if they can be rewrite to avoid the complication.
Well, I think there might be some tradeoff here. On the one hand, use the same register 't1' for two different purposes (scratch register & flags registers) at the same time looks rather error prone as mentioned in the PR description. On the other hand, use another register 't2' as the dedicated flags register could somehow affect the performance in certain cases even though this is not reflected on the specjbb2015 numbers. Personally, I have no obvious bias here. I would like to hear how the other Reviewers would say about this. Maybe @shipilev ? Thanks. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/21
On Thu, 2 Dec 2021 04:02:02 GMT, Fei Yang <fyang@openjdk.org> wrote:
Hi Yadong, it looks cost too much to reserve a dedicated register for control flag. I think in most cases we can use the 2 scratch registers instead. Can you show some complicated cases? We can check if they can be rewrite to avoid the complication.
Well, I think there might be some tradeoff here. On the one hand, use the same register 't1' for two different purposes (scratch register & flags registers) at the same time looks rather error prone as mentioned in the PR description. On the other hand, use another register 't2' as the dedicated flags register could somehow affect the performance in certain cases even though this is not reflected on the specjbb2015 numbers. Personally, I have no obvious bias here. I would like to hear how the other Reviewers would say about this. Maybe @shipilev ? Thanks.
Agree. it's a tradeoff. There may be more spill/unspills in some scenarios of a high register pressue, but it is more friendly for code scheduling and higher maintainability with less data flow effects. By the way, we found this PR did not affect the performance of SPECjvm2008 on unmatched. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/21
On Thu, 2 Dec 2021 13:04:06 GMT, Yadong Wang <yadongwang@openjdk.org> wrote:
Hi Yadong, it looks cost too much to reserve a dedicated register for control flag. I think in most cases we can use the 2 scratch registers instead. Can you show some complicated cases? We can check if they can be rewrite to avoid the complication.
All instructs that remove the rFlagsReg operand are implicitly use t1. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/21
On Thu, 2 Dec 2021 13:04:06 GMT, Yadong Wang <yadongwang@openjdk.org> wrote:
Hi Yadong, it looks cost too much to reserve a dedicated register for control flag. I think in most cases we can use the 2 scratch registers instead. Can you show some complicated cases? We can check if they can be rewrite to avoid the complication.
Well, I think there might be some tradeoff here. On the one hand, use the same register 't1' for two different purposes (scratch register & flags registers) at the same time looks rather error prone as mentioned in the PR description. On the other hand, use another register 't2' as the dedicated flags register could somehow affect the performance in certain cases even though this is not reflected on the specjbb2015 numbers. Personally, I have no obvious bias here. I would like to hear how the other Reviewers would say about this. Maybe @shipilev ? Thanks.
Agree. it's a tradeoff. There may be more spill/unspills in some scenarios of a high register pressue, but it is more friendly for code scheduling and higher maintainability with less data flow effects. By the way, we found this PR did not affect the performance of SPECjvm2008 on unmatched:
beforeļ¼
``` compress 17.71 crypto 24.16 derby 32.39 mpegaudio 14.68 scimark.large 3.65 scimark.small 15.07 serial 12.08 startup 3.88 sunflow 7.21 ```
after:
``` compress 17.74 crypto 24.1 derby 32.67 mpegaudio 14.64 scimark.large 3.67 scimark.small 15.42 serial 12.98 startup 3.6 sunflow 7.09 ```
Hi Yadong, Thanks for the test. Since benchmark score showed little impact so far. I'm fine to use a dedicated register now. But it still looks too many special registers in riscv. I think it can be improved in future. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/21
On Wed, 1 Dec 2021 09:35:26 GMT, Yadong Wang <yadongwang@openjdk.org> wrote:
There are non-allocatable registers t0 and t1 in C2, frequently used as temporary registers for common code snipptes. Meanwhile, C2 use t1 as the flags register, so that many instructs using t1 as a temporary register must declare the data flow effect of t1 which is not part of a match rule. So we can reserve t2 and use it as the machine flags register in C2 instead, to reduce the probability of misuse of t1 without declaring the data flow effect.
Yadong Wang has updated the pull request incrementally with one additional commit since the last revision:
8278041: riscv: Use t2 as the dedicated machine flags register in C2(2)
Let's keep the current implementation for some time to see :-) ------------- PR: https://git.openjdk.java.net/riscv-port/pull/21
On Wed, 1 Dec 2021 09:35:26 GMT, Yadong Wang <yadongwang@openjdk.org> wrote:
There are non-allocatable registers t0 and t1 in C2, frequently used as temporary registers for common code snipptes. Meanwhile, C2 use t1 as the flags register, so that many instructs using t1 as a temporary register must declare the data flow effect of t1 which is not part of a match rule. So we can reserve t2 and use it as the machine flags register in C2 instead, to reduce the probability of misuse of t1 without declaring the data flow effect.
Yadong Wang has updated the pull request incrementally with one additional commit since the last revision:
8278041: riscv: Use t2 as the dedicated machine flags register in C2(2)
I'll close this pr first, and reopen later if needed. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/21
On Wed, 1 Dec 2021 07:56:54 GMT, Yadong Wang <yadongwang@openjdk.org> wrote:
There are non-allocatable registers t0 and t1 in C2, frequently used as temporary registers for common code snipptes. Meanwhile, C2 use t1 as the flags register, so that many instructs using t1 as a temporary register must declare the data flow effect of t1 which is not part of a match rule. So we can reserve t2 and use it as the machine flags register in C2 instead, to reduce the probability of misuse of t1 without declaring the data flow effect.
This pull request has been closed without being integrated. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/21
participants (3)
-
Fei Yang
-
kuaiwei
-
Yadong Wang