Dump register variables correctly #2

ghost · 2021-09-17T15:54:29Z

No description provided.

antoyo · 2021-09-25T21:59:30Z

I missed this PR somehow.

Is this necessary for you now?

My workflow of submitting gcc patches is cumbersome, so I'm not sure how I'll handle external contribution to those patches yet.

ghost · 2021-09-26T02:06:15Z

Not really, just a little thing. It was more of "let's finally take a look at GCC internals, shall we?", and then I got distracted again and forgot about it.

My workflow of submitting gcc patches is cumbersome, so I'm not sure how I'll handle external contribution to those patches yet.

Oh, I was planning to send more while working (for example) on rust-lang/rustc_codegen_gcc#87 . Could that be a problem?

antoyo · 2021-09-26T13:36:32Z

If you send a PR that is completely independent of the other patches that should be relatively easy for me to handle it eventually.
I'll probably not send it for review until all the previous ones are merged if it adds a new API because I don't want to manage the conflicts in the libgccjit.map file.

Please read the guidelines in order to not forget anything that should be done for a gcc patch.
If you have any trouble for stuff like how to check the formatting is right, I can help you.

Also, if you are interested in sending your patches yourself to gcc, note that you don't need the copyright assignment anymore.

ghost · 2021-09-26T15:36:42Z

I sent this PR here and not to the mailing list because this patch is just a little extension to your patch. I'd be super happy to transfer these poor 5 LOC to public domain so you could just attribute them to yourself.

If you have any trouble for stuff like how to check the formatting is right, I can help you.

Yeah, please help. I see GCC uses some mix of tabs and spaces that resembles "indent with tabs (width 8), pad with spaces" scheme, but sometimes it's spaces only. I'm lost. Any place this scheme is written down to?

Also, if you are interested in sending your patches yourself to gcc, note that you don't need the copyright assignment anymore.

What's copyright assignment?

antoyo · 2021-09-26T19:27:17Z

Yeah, please help. I see GCC uses some mix of tabs and spaces that resembles "indent with tabs (width 8), pad with spaces" scheme, but sometimes it's spaces only. I'm lost. Any place this scheme is written down to?

There are two scripts that I use:

./contrib/gcc-changelog/git_check_commit.py

and

./contrib/check_GNU_style.sh 0001-patch-name.patch

What's copyright assignment?

GCC used to require that you give the copyright to the FSF and you had to sign a document.
Now, it's not required anymore, so it should be easier for you if you want to contribute directly.

Otherwise, I can always send your patches for you, but I'd appreciate if they pass the above tests.

DR 2352 changed the definitions of reference-related (so that it uses "similar type" instead of "same type") and of reference-compatible (use a standard conversion sequence). That means that reference-related is now more broad, which means that we will be binding more things directly. The original patch for DR 2352 caused some problems, which were fixed in r276251 by creating a "fake" ck_qual in direct_reference_binding, so that in void f(int *); // #1 void f(const int * const &); // #2 int *x; int main() { f(x); // call #1 } we call #1. The extra ck_qual in #2 causes compare_ics to select #1, which is a better match for "int *" because then we don't have to do a qualification conversion. Let's turn to the problem in this PR. We have void f(const int * const &); // #1 void f(const int *); // #2 int *x; int main() { f(x); } We arrive in compare_ics to decide which one is better. The ICS for #1 looks like ck_ref_bind <- ck_qual <- ck_identity const int *const & const int *const int * and the ICS for #2 is ck_qual <- ck_rvalue <- ck_identity const int * int * int * We strip the reference and then comp_cv_qual_signature when comparing two ck_quals sees that "const int *" is a proper subset of "const int *const" and we return -1. But that's wrong; presumably the top-level "const" should be ignored and the call should be ambiguous. This patch adjust the type of the "fake" ck_qual so that this problem doesn't arise. PR c++/97296 gcc/cp/ChangeLog: * call.cc (direct_reference_binding): strip_top_quals when creating a ck_qual. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/ref-bind4.C: Add dg-error. * g++.dg/cpp0x/ref-bind8.C: New test.

Improve stack protector patterns and peephole2s even more: a. Use unrelated register clears with integer mode size <= word mode size to clear stack protector scratch register. b. Use unrelated register initializations in front of stack protector sequence to clear stack protector scratch register. c. Use unrelated register initializations using LEA instructions to clear stack protector scratch register. These stack protector improvements reuse 6914 unrelated register initializations to substitute the clear of stack protector scratch register in 12034 instances of stack protector sequence in recent linux defconfig build. gcc/ChangeLog: * config/i386/i386.md (@stack_protect_set_1_<PTR:mode>_<W:mode>): Use W mode iterator instead of SWI48. Output MOV instead of XOR for TARGET_USE_MOV0. (stack_protect_set_1 peephole2): Use integer modes with mode size <= word mode size for operand 3. (stack_protect_set_1 peephole2 rust-lang#2): New peephole2 pattern to substitute stack protector scratch register clear with unrelated register initialization, originally in front of stack protector sequence. (*stack_protect_set_3_<PTR:mode>_<SWI48:mode>): New insn pattern. (stack_protect_set_1 peephole2): New peephole2 pattern to substitute stack protector scratch register clear with unrelated register initialization involving LEA instruction.

Use unrelated register initializations using zero/sign-extend instructions to clear stack protector scratch register. Hanlde only SI -> DImode extensions for 64-bit targets, as this is the only extension that triggers the peephole in a non-negligible number. Also use explicit check for word_mode instead of mode iterator in peephole2 patterns to avoid pattern explosion. gcc/ChangeLog: * config/i386/i386.md (stack_protect_set_1 peephole2): Explicitly check operand 2 for word_mode. (stack_protect_set_1 peephole2 rust-lang#2): Ditto. (stack_protect_set_2 peephole2): Ditto. (stack_protect_set_3 peephole2): Ditto. (*stack_protect_set_4z_<mode>_di): New insn patter. (*stack_protect_set_4s_<mode>_di): Ditto. (stack_protect_set_4 peephole2): New peephole2 pattern to substitute stack protector scratch register clear with unrelated register initialization involving zero/sign-extend instruction.

This patch adjusts the costs so that we treat REG and SUBREG expressions the same for costing. This was motivated by bt_skip_func and bt_find_func in xz and results in nearly a 5% improvement in the dynamic instruction count for input #2 and smaller, but definitely visible improvements pretty much across the board. Exceptions would be perlbench input #1 and exchange2 which showed very small regressions. In the bt_find_func and bt_skip_func cases we have something like this: > (insn 10 7 11 2 (set (reg/v:DI 136 [ x ]) > (zero_extend:DI (subreg/s/u:SI (reg/v:DI 137 [ a ]) 0))) "zz.c":6:21 387 {*zero_extendsidi2_bitmanip} > (nil)) > (insn 11 10 12 2 (set (reg:DI 142 [ _1 ]) > (plus:DI (reg/v:DI 136 [ x ]) > (reg/v:DI 139 [ b ]))) "zz.c":7:23 5 {adddi3} > (nil)) [ ... ]> (insn 13 12 14 2 (set (reg:DI 143 [ _2 ]) > (plus:DI (reg/v:DI 136 [ x ]) > (reg/v:DI 141 [ c ]))) "zz.c":8:23 5 {adddi3} > (nil)) Note the two uses of (reg 136). The best way to handle that in combine might be a 3->2 split. But there's a much better approach if we look at fwprop... (set (reg:DI 142 [ _1 ]) (plus:DI (zero_extend:DI (subreg/s/u:SI (reg/v:DI 137 [ a ]) 0)) (reg/v:DI 139 [ b ]))) change not profitable (cost 4 -> cost 8) So that should be the same cost as a regular DImode addition when the ZBA extension is enabled. But it ends up costing more because the clause to cost this variant isn't prepared to handle a SUBREG. That results in the RTL above having too high a cost and fwprop gives up. One approach would be to replace the REG_P with REG_P || SUBREG_P in the costing code. I ultimately decided against that and instead check if the operand in question passes register_operand. By far the most important case to handle is the DImode PLUS. But for the sake of consistency, I changed the other instances in riscv_rtx_costs as well. For those other cases we're talking about improvements in the .000001% range. While we are into stage4, this just hits cost modeling which we've generally agreed is still appropriate (though we were mostly talking about vector). So I'm going to extend that general agreement ever so slightly and include scalar cost modeling :-) gcc/ * config/riscv/riscv.cc (riscv_rtx_costs): Handle SUBREG and REG similarly. gcc/testsuite/ * gcc.target/riscv/reg_subreg_costs.c: New test. Co-authored-by: Jivan Hakobyan <[email protected]>

We evaluate constexpr functions on the original, pre-genericization bodies. That means that the function body we're evaluating will not have gone through cp_genericize_r's "Map block scope extern declarations to visible declarations with the same name and type in outer scopes if any". Here: constexpr bool bar() { return true; } // #1 constexpr bool foo() { constexpr bool bar(void); // rust-lang#2 return bar(); } it means that we: 1) register_constexpr_fundef (#1) 2) cp_genericize (#1) nothing interesting happens 3) register_constexpr_fundef (foo) does copy_fn, so we have two copies of the BIND_EXPR 4) cp_genericize (foo) this remaps rust-lang#2 to #1, but only on one copy of the BIND_EXPR 5) retrieve_constexpr_fundef (foo) we find it, no problem 6) retrieve_constexpr_fundef (rust-lang#2) and here rust-lang#2 isn't found in constexpr_fundef_table, because we're working on the BIND_EXPR copy where rust-lang#2 wasn't mapped to #1 so we fail. We've only registered #1. It should work to use DECL_LOCAL_DECL_ALIAS (which used to be extern_decl_map). We evaluate constexpr functions on pre-cp_fold bodies to avoid diagnostic problems, but the remapping I'm proposing should not interfere with diagnostics. This is not a problem for a global scope redeclaration; there we go through duplicate_decls which keeps the DECL_UID: DECL_UID (olddecl) = olddecl_uid; and DECL_UID is what constexpr_fundef_hasher::hash uses. PR c++/111132 gcc/cp/ChangeLog: * constexpr.cc (get_function_named_in_call): Use cp_get_fndecl_from_callee. * cvt.cc (cp_get_fndecl_from_callee): If there's a DECL_LOCAL_DECL_ALIAS, use it. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/constexpr-redeclaration3.C: New test. * g++.dg/cpp0x/constexpr-redeclaration4.C: New test.

Dump register variables correctly

044dde7

antoyo force-pushed the master branch from 5182fae to 8169963 Compare December 16, 2021 03:00

antoyo force-pushed the master branch from 8169963 to 2867bc3 Compare January 9, 2022 18:47

antoyo force-pushed the master branch from 13789ac to ad92c62 Compare January 27, 2022 14:15

antoyo force-pushed the master branch from 9687f69 to c689be4 Compare June 24, 2022 19:31

antoyo force-pushed the master branch from 7cfbde4 to e7f055b Compare August 29, 2022 00:57

antoyo force-pushed the master branch 3 times, most recently from 0a94141 to 17858b5 Compare December 8, 2022 23:45

antoyo force-pushed the master branch from 17858b5 to b518f40 Compare December 14, 2022 05:32

antoyo force-pushed the master branch from bb21645 to c5c3a0d Compare January 9, 2023 03:15

antoyo force-pushed the master branch from f858ad7 to 332ad79 Compare February 3, 2023 18:24

antoyo force-pushed the master branch from 332ad79 to c97bc65 Compare March 4, 2023 05:45

antoyo force-pushed the master branch 9 times, most recently from 648c851 to 16686cb Compare August 24, 2023 20:31

antoyo force-pushed the master branch 3 times, most recently from 8ecf518 to ed50105 Compare February 3, 2024 17:46

antoyo force-pushed the master branch 4 times, most recently from 51fa28a to be000af Compare February 9, 2024 14:12

antoyo force-pushed the master branch 4 times, most recently from cdd8978 to ad4ffde Compare February 16, 2024 21:34

antoyo force-pushed the master branch 2 times, most recently from cf95541 to b6f163f Compare March 1, 2024 15:35

antoyo force-pushed the master branch from 5939627 to 8620316 Compare April 26, 2024 13:39

antoyo force-pushed the master branch 2 times, most recently from e744a94 to bb9fe7d Compare September 28, 2024 13:44

antoyo force-pushed the master branch 2 times, most recently from 9cec8ab to c4ee893 Compare October 3, 2024 21:55

antoyo force-pushed the master branch 6 times, most recently from 1e817bd to b4002fd Compare October 17, 2024 14:21

antoyo force-pushed the master branch 4 times, most recently from 2e49eb4 to 85e56c5 Compare November 14, 2024 18:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dump register variables correctly #2

Dump register variables correctly #2

ghost commented Sep 17, 2021

antoyo commented Sep 25, 2021

ghost commented Sep 26, 2021

antoyo commented Sep 26, 2021

ghost commented Sep 26, 2021

antoyo commented Sep 26, 2021

Dump register variables correctly #2

Are you sure you want to change the base?

Dump register variables correctly #2

Conversation

ghost commented Sep 17, 2021

antoyo commented Sep 25, 2021

ghost commented Sep 26, 2021

antoyo commented Sep 26, 2021

ghost commented Sep 26, 2021

antoyo commented Sep 26, 2021