Re: [pushed][PATCH v3] LoongArch: Avoid non-returning indirect jumps through $ra [PR110136]
Hi, On 6/15/23 17:03, Xi Ruoyao wrote: Xuerui: I guess this makes it sensible to show "ret" instead of "jirl $zero, $ra, 0" in objdump -d output, but I don't know how to implement it. Do you have some idea? Thanks for the suggestion! Actually I have previously made this patch series [1] which included just that. But the Loongson maintainers said they're working on linker relaxation at that time so they would have to postpone processing it, and I've never had a review since then; it's expected to conflict with the relaxation patches so some rebasing would be needed, but IIRC all review comments should be addressed. You can take the series if you'd like to ;-) [1]: https://sourceware.org/pipermail/binutils/2023-February/126088.html On Thu, 2023-06-15 at 16:27 +0800, Lulu Cheng wrote: Pushed to trunk and gcc-12 gcc-13. r14-1866 r13-7448 r12-9698 在 2023/6/15 上午9:30, Lulu Cheng 写道: Micro-architecture unconditionally treats a "jr $ra" as "return from subroutine", hence doing "jr $ra" would interfere with both subroutine return prediction and the more general indirect branch prediction. Therefore, a problem like PR110136 can cause a significant increase in branch error prediction rate and affect performance. The same problem exists with "indirect_jump". gcc/ChangeLog: * config/loongarch/loongarch.md: Modify the register constraints for template "jumptable" and "indirect_jump" from "r" to "e". Co-authored-by: Andrew Pinski --- v1 -> v2: 1. Modify the description. 2. Modify the register constraints of the template "indirect_jump". v2 -> v3: 1. Modify the description. --- gcc/config/loongarch/loongarch.md | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gcc/config/loongarch/loongarch.md b/gcc/config/loongarch/loongarch.md index 816a943d155..b37e070660f 100644 --- a/gcc/config/loongarch/loongarch.md +++ b/gcc/config/loongarch/loongarch.md @@ -2895,6 +2895,10 @@ (define_insn "*jump_pic" } [(set_attr "type" "branch")]) +;; Micro-architecture unconditionally treats a "jr $ra" as "return from subroutine", +;; non-returning indirect jumps through $ra would interfere with both subroutine +;; return prediction and the more general indirect branch prediction. + (define_expand "indirect_jump" [(set (pc) (match_operand 0 "register_operand"))] "" @@ -2905,7 +2909,7 @@ (define_expand "indirect_jump" }) (define_insn "@indirect_jump" - [(set (pc) (match_operand:P 0 "register_operand" "r"))] + [(set (pc) (match_operand:P 0 "register_operand" "e"))] "" "jr\t%0" [(set_attr "type" "jump") @@ -2928,7 +2932,7 @@ (define_expand "tablejump" (define_insn "@tablejump" [(set (pc) - (match_operand:P 0 "register_operand" "r")) + (match_operand:P 0 "register_operand" "e")) (use (label_ref (match_operand 1 "" "")))] "" "jr\t%0"
Re: [pushed][PATCH v3] LoongArch: Avoid non-returning indirect jumps through $ra [PR110136]
Xuerui: I guess this makes it sensible to show "ret" instead of "jirl $zero, $ra, 0" in objdump -d output, but I don't know how to implement it. Do you have some idea? On Thu, 2023-06-15 at 16:27 +0800, Lulu Cheng wrote: > Pushed to trunk and gcc-12 gcc-13. > r14-1866 > r13-7448 > r12-9698 > > 在 2023/6/15 上午9:30, Lulu Cheng 写道: > > Micro-architecture unconditionally treats a "jr $ra" as "return from > > subroutine", > > hence doing "jr $ra" would interfere with both subroutine return > > prediction and > > the more general indirect branch prediction. > > > > Therefore, a problem like PR110136 can cause a significant increase > > in branch error > > prediction rate and affect performance. The same problem exists with > > "indirect_jump". > > > > gcc/ChangeLog: > > > > * config/loongarch/loongarch.md: Modify the register > > constraints for template > > "jumptable" and "indirect_jump" from "r" to "e". > > > > Co-authored-by: Andrew Pinski > > --- > > v1 -> v2: > > 1. Modify the description. > > 2. Modify the register constraints of the template > > "indirect_jump". > > v2 -> v3: > > 1. Modify the description. > > --- > > gcc/config/loongarch/loongarch.md | 8 ++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/gcc/config/loongarch/loongarch.md > > b/gcc/config/loongarch/loongarch.md > > index 816a943d155..b37e070660f 100644 > > --- a/gcc/config/loongarch/loongarch.md > > +++ b/gcc/config/loongarch/loongarch.md > > @@ -2895,6 +2895,10 @@ (define_insn "*jump_pic" > > } > > [(set_attr "type" "branch")]) > > > > +;; Micro-architecture unconditionally treats a "jr $ra" as "return > > from subroutine", > > +;; non-returning indirect jumps through $ra would interfere with > > both subroutine > > +;; return prediction and the more general indirect branch > > prediction. > > + > > (define_expand "indirect_jump" > > [(set (pc) (match_operand 0 "register_operand"))] > > "" > > @@ -2905,7 +2909,7 @@ (define_expand "indirect_jump" > > }) > > > > (define_insn "@indirect_jump" > > - [(set (pc) (match_operand:P 0 "register_operand" "r"))] > > + [(set (pc) (match_operand:P 0 "register_operand" "e"))] > > "" > > "jr\t%0" > > [(set_attr "type" "jump") > > @@ -2928,7 +2932,7 @@ (define_expand "tablejump" > > > > (define_insn "@tablejump" > > [(set (pc) > > - (match_operand:P 0 "register_operand" "r")) > > + (match_operand:P 0 "register_operand" "e")) > > (use (label_ref (match_operand 1 "" "")))] > > "" > > "jr\t%0" > -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University
Re: [pushed][PATCH v3] LoongArch: Avoid non-returning indirect jumps through $ra [PR110136]
Pushed to trunk and gcc-12 gcc-13. r14-1866 r13-7448 r12-9698 在 2023/6/15 上午9:30, Lulu Cheng 写道: Micro-architecture unconditionally treats a "jr $ra" as "return from subroutine", hence doing "jr $ra" would interfere with both subroutine return prediction and the more general indirect branch prediction. Therefore, a problem like PR110136 can cause a significant increase in branch error prediction rate and affect performance. The same problem exists with "indirect_jump". gcc/ChangeLog: * config/loongarch/loongarch.md: Modify the register constraints for template "jumptable" and "indirect_jump" from "r" to "e". Co-authored-by: Andrew Pinski --- v1 -> v2: 1. Modify the description. 2. Modify the register constraints of the template "indirect_jump". v2 -> v3: 1. Modify the description. --- gcc/config/loongarch/loongarch.md | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gcc/config/loongarch/loongarch.md b/gcc/config/loongarch/loongarch.md index 816a943d155..b37e070660f 100644 --- a/gcc/config/loongarch/loongarch.md +++ b/gcc/config/loongarch/loongarch.md @@ -2895,6 +2895,10 @@ (define_insn "*jump_pic" } [(set_attr "type" "branch")]) +;; Micro-architecture unconditionally treats a "jr $ra" as "return from subroutine", +;; non-returning indirect jumps through $ra would interfere with both subroutine +;; return prediction and the more general indirect branch prediction. + (define_expand "indirect_jump" [(set (pc) (match_operand 0 "register_operand"))] "" @@ -2905,7 +2909,7 @@ (define_expand "indirect_jump" }) (define_insn "@indirect_jump" - [(set (pc) (match_operand:P 0 "register_operand" "r"))] + [(set (pc) (match_operand:P 0 "register_operand" "e"))] "" "jr\t%0" [(set_attr "type" "jump") @@ -2928,7 +2932,7 @@ (define_expand "tablejump" (define_insn "@tablejump" [(set (pc) - (match_operand:P 0 "register_operand" "r")) + (match_operand:P 0 "register_operand" "e")) (use (label_ref (match_operand 1 "" "")))] "" "jr\t%0"
