Re: syncing the GCC vax port
Sorry for the delay... Updated diff: http://coypu.sdf.org/vax-gcc10.diff On Mon, Apr 29, 2019 at 02:08:32PM -0600, Jeff Law wrote: > On 3/30/19 3:03 AM, co...@sdf.org wrote: > > hi folks, > > > > i was interesting in tackling some problems gcc netbsd/vax has. > > it has some ICEs which are in reload phase. searching around, the answer > > to that is "switch to LRA first". Now, I don't quite know what that is > > yet, but I know I need to try to do it. > > > > As an initial step, I need to sync the source code. > > netbsd/vax has some outstanding work on GCC. > > > > I've done this, and I can run programs built by this compiler: > > http://coypu.sdf.org/gcc-9-vax.diff > > > > (My tree has more detail on the changes done: > > https://github.com/gcc-mirror/gcc/compare/master...coypoop:vax ) > > > > Matt Thomas (the GCC VAX maintainer) is the author of most of these > > changes, I suspect he will not be very responsive by email. > > Not being the author, I might not be able to answer all the questions, > > but I can try my best. > > > > How do I get this across? comments? straight to gcc-patches? :-) > > > > I know Jeff Law did not like the change to builtins.md as being wrong. I > > can omit them, I forgot about it until typing this email :) > > > > caveat: had an ICE during reload in the build process, I hid it > > under the rug with -O0. > I don't recall what I objected to :-) From looking at the changes I'd > be concerned about the changes from memory_operand to > volatile_mem_operand. Without knowing more about the problem you're > trying to solve it's hard to ACK something like this. Removed from the diff. Unfortunately this introduces an ICE during the build of GCC (but that will be the second kind) > The new "condjump" expander seems a bit under-specified. Is there some > reason why you needed that expander and couldn't just add a name to a > existing define_insn? I didn't find an answer for this yet. What I got so far: Apparently vax did not include builtins.md before, and so an adjustment that should have happened when the cond-optab branch was merged didn't happen. > A nit. In that expander you have "gen_rtx_NE(VOIDmode ..." There > should be a space between the NE and the open parenthesis. That kind of > nit is repeated several times. Fixed. > The changes in constraints.md do not seem to change functionality at > all, just reordering the oprerands. However our preferred style is > what's in there right now. Undid that. > ival >= 0 is the right way 0 <= ival is the wrong way. Fixed. > I noticed the patch turns off flag_dwarf2_cfi_asm. Is there a reason > for that? But yet you create DEF_CFA notes in the prologue. Is it the > case that the CFI bits just aren't ready for consumption yet? If not, > then it may be easier to just not include those changes right now. This is due to a binutils issue. It looks similar to https://github.com/riscv/riscv-binutils-gdb/issues/76 I have yet to find a fix for it. > I can't really comment on the changes to how addreses are handled in > print_operand_address. I'd just have to assume they're correct. > > I don't know if we generally allow debug_rtx calls like are added. > Usually we gcc_assert or gcc_unreachable. OK. I removed those and added a single gcc_unreachable. > mkrtx needs a function comment and looks like its got some formatting > problems (spaces vs tabs issues and missing space between function name > an the open paren for arguments). Similarly for vax_output_movmemsi and > legitimate_pic_operand_p, vax_decomposed_dimode_operand_p, Added comments. > You've got undocumented #ifdefs in legitimate_pic_operand_p. elf.h contains: /* Don't allow *foo which foo is non-local */ #define NO_EXTERNAL_INDIRECT_ADDRESS Is this sufficient? thanks. > You've got incorrect operand ordering in the adjacent_operands_p changes. Fixed. > The netbsd specific changes to the zero_extract patterns looks strange. > Why why not just have the right operand. And why change it in the > first place? Refers to same as first comment: I removed these changes. Back to the drawing board with that crash. > Those peepholes look strange. Why is the first insn not just removed as > dead? I don't understand this comment (sorry). Can you clarify it? I removed one peephole that was (0 &&...), if that helps. > And if these changes were done by Matt Thomas, does he have a copyright > assignment on file? If not, then we can't really use them. These are > large enough that they'd require an assignment. Yes, Matt Thomas has a copyright assignment on file.
Atomics in C++11
Greetings, I was wondering if its possible to use the C11 atomics library for multithreading GCC. Not sure if its a good idea due to concerns about older plaforms not having a C11 supported libraries or compiler. If not then the best way is pthreads key support and other posix thread supports. Thanks, Nick
Re: [testsuite] Effective-target depending on current compiler flags?
On Wed, 11 Sep 2019 at 14:21, Christophe Lyon
wrote:
>
> On Wed, 11 Sep 2019 at 11:56, Richard Sandiford
> wrote:
> >
> > Christophe Lyon writes:
> > > Hi,
> > >
> > > While looking at GCC validation results when configured for ARM
> > > cortex-m33 by default, I noticed that
> > > FAIL: gcc.target/arm/cmse/mainline/soft/cmse-5.c -march=armv8-m.main
> > > -mthumb -mfloat-abi=soft -O0 scan-assembler msr\tAPSR_nzcvqg, lr
> > >
> > > The corresponding line in the testcase is (are):
> > > /* { dg-final { scan-assembler "msr\tAPSR_nzcvq, lr" { target { !
> > > arm_dsp } } } } */
> > > /* { dg-final { scan-assembler "msr\tAPSR_nzcvqg, lr" { target arm_dsp }
> > > } } */
> > >
> > > So the arm_dsp effective target is true and the test tries to match
> > > APSR_nzcvqg, while APSR_nzcvq is generated, as expected.
> > >
> > > There is an inconsistency because the testcase is compiled with
> > > -march=armv8-m.main, while arm_dsp is not: like most effective
> > > targets, it does not take the current cflags into account.
> > > In the present case, the -march flag is added by cmse.exp in the
> > > arguments to gcc-dg-runtest.
> > >
> > > I tried to add [current_compiler_flags] to all arm effective targets
> > > (for consistency, it wouldn't make sense to add it to arm_dsp only),
> > > but then I noticed further problems...
> >
> > Yeah, effective targets shouldn't depend on the compiler flags.
> > They're supposed to be properties of the testing target (what it
> > supports, what it enables by default, etc.) and are cached between
> > tests that run with different options.
> >
> Thanks for the clarification/confirmation it currently works as intended.
Actually I've just realized that effective targets also depend on
compiler flags passed via RUNTESTFLAGS / -target-board, so this is
getting tricky...
>
> > CMSE isn't my area, so I don't know why the scan-assembler lines
> > were written this way. Is the { target arm_dsp } line there for
> > cases in which a user-specified -march flag manages to override
> > -march=armv8-m.main?
> >
> I've added Andre in cc:, as he originally wrote that test.
>
If I add -march=armv8-m.main+dsp via RUNTESTFLAGS/-target-board,
arm_dsp succeeds, but the testcase (cmse-5.c) is compiled with
-march=armv8-m.main+dsp [...] -march=armv8-m.main
the last flag comes from cmse.exp via add_options_for_arm_arch_v8m_main
But IIRC the order between RUNTESTFLAGS and dg-options depends on the
dejagnu version (somewhere near 1.6)
Anyway, this makes me wonder what's the supposed/recommended way of
running validations for non-default cpu?
1- configure --with-cpu=cortex-XX ; make check
2- configure (using default cpu setting) ;
RUNTESTFLAGS=-mcpu=cortex-XX make check
I use (1), but I think most others use (2) ?
Thanks,
Christophe
> Thanks,
>
> Christophe
>
> > Thanks,
> > Richard
> >
> > > For instance, in my configuration, when advsimd-intrinsics.exp is
> > > executed, it tries
> > > check_effective_target_arm_v8_2a_fp16_neon_hw
> > > which fails, and then tries check_effective_target_arm_neon_fp16_ok
> > > which succeeds and thus adds -mfloat-abi=softfp -mfp16-format=ieee to
> > > additional_flags.
> > >
> > > Since $additional_flags is passed to gcc-dg-runtest, these flags are
> > > visible by current_compiler_flags and taken into account when
> > > computing effective_targets (since I modified them).
> > >
> > > So... when computing arm_v8_2a_fp16_scalar_ok, it uses
> > > -mfloat-abi=softfp -mfp16-format=ieee and doesn't need to add any flag
> > > beyond -march=armv8.2-a+fp16.
> > > So et_arm_v8_2a_fp16_scalar_flags contains -march=armv8.2-a+fp16 only.
> > >
> > > Later on, while executing arm.exp, -mfloat-abi=softfp
> > > -mfp16-format=ieee are not part of $DEFAULT_CFLAGS as used by
> > > dg-runtest, but et_arm_v8_2a_fp16_scalar_flags is still in the cache
> > > and it's not valid anymore.
> > >
> > > So. before burying myself, is there already a way to make
> > > effective-targets take the current compiler flags into account as
> > > needed in gcc.target/arm/cmse/mainline/soft/cmse-5.c ?
> > >
> > > Not sure my explanation is clear enough :-)
> > >
> > > Thanks,
> > >
> > > Christophe
Re: Proposal for the transition timetable for the move to GIT
On 9/17/19 6:02 AM, Richard Earnshaw (lists) wrote: > At the Cauldron this weekend the overwhelming view for the move to GIT > soon was finally expressed. [ ... proposal itself ... ] So there's nothing in the proposal I would object to, nor do I object to being slightly flexible. If we need to move the transition a few days into the new year because of developer availability, that seems fine. Similarly if we want to move up the date for a decision to be made that's fine as well so long as the potentially affected parties are notified ASAP what that date is. With the SVN repo going read-only it becomes our fallback plan in case of major unexpected problems. Joseph's recommendation for having the old objects/refs in the new repo makes a lot of sense. So if it works, it's got my support as well. Anyway, just wanted to chime in with my support for the plan and make it clear that as long as we get a conversion that is as good as or better than the mirror is now that I'll be happy :-) jeff
Re: taking OpenCL C as a built-in lang of GCC?
On 9/12/19 8:48 AM, Jianbin Fang wrote: > Hello Guys, > > > > I am working on OpenCL for a couple of years, and would like to ask, > as for GCC, why not taking OpenCL C as a built-in language in its > front-end? There's no inherent reason why we don't support OpenCL C. Someone would just need to write a suitable front-end for GCC and contribute it. jeff
Re: Adding -Wshadow=local to gcc build rules
On 9/19/19 12:19 PM, Richard Biener wrote:
> On Wed, Sep 18, 2019 at 3:09 PM Bernd Edlinger
> wrote:
>>
>> Hi,
>>
>> I'm currently trying to add -Wshadow=local to the gcc build rules.
>> I started with -Wshadow, but gave up that idea immediately.
>>
>> As you could expect the current code base has plenty of shadowed
>> local variables. Most are trivial to resolve, some are less trivial.
>> I am not finished yet, but it is clear that it will be a rather big
>> patch.
>>
>> I would like to ask you if you agree that would be a desirable step,
>> in improving code quality in the gcc tree.
>
> I wonder if -Wshadow=compatible-local is easier to achieve?
>
I tried that and it does not make a big difference, while
it misses for instance:
* gcc/c-family/c-ada-spec.c (dump_ada_macros)
unsigned char *s, shadowed by const unsigned char *s. :-/
On the other hand, -Wshadow=global is a lot more difficult,
because we have lots of globals, for instance:
context.h:
/* The global singleton context aka "g".
(the name is chosen to be easy to type in a debugger). */
extern gcc::context *g;
But unfortunately Wshadow=local does not find class members
shadowed by local variable, which happens for instance in
wide-int
With -Wshadow, I had to change this in wide-int.h:
Index: gcc/wide-int.h
===
--- gcc/wide-int.h (revision 275545)
+++ gcc/wide-int.h (working copy)
@@ -648,9 +648,9 @@ namespace wi
storage_ref () {}
storage_ref (const HOST_WIDE_INT *, unsigned int, unsigned int);
-const HOST_WIDE_INT *val;
-unsigned int len;
-unsigned int precision;
+const HOST_WIDE_INT *m_val;
+unsigned int m_len;
+unsigned int m_precision;
So this change was not necessary for -Wshadow=local, although
I would think that, shadowing class members is not much different from
shadowing a local scope.
Not sure if shadowing class members should be part of -Wshadow=local
instead of -Wshadow=global.
Bernd.
Re: Adding -Wshadow=local to gcc build rules
On Fri, Sep 20, 2019, 2:21 PM Bernd Edlinger
wrote:
> On 9/19/19 12:19 PM, Richard Biener wrote:
> > On Wed, Sep 18, 2019 at 3:09 PM Bernd Edlinger
> > wrote:
> >>
> >> Hi,
> >>
> >> I'm currently trying to add -Wshadow=local to the gcc build rules.
> >> I started with -Wshadow, but gave up that idea immediately.
> >>
> >> As you could expect the current code base has plenty of shadowed
> >> local variables. Most are trivial to resolve, some are less trivial.
> >> I am not finished yet, but it is clear that it will be a rather big
> >> patch.
> >>
> >> I would like to ask you if you agree that would be a desirable step,
> >> in improving code quality in the gcc tree.
> >
> > I wonder if -Wshadow=compatible-local is easier to achieve?
> >
>
> I tried that and it does not make a big difference, while
> it misses for instance:
>
> * gcc/c-family/c-ada-spec.c (dump_ada_macros)
> unsigned char *s, shadowed by const unsigned char *s. :-/
>
> On the other hand, -Wshadow=global is a lot more difficult,
> because we have lots of globals, for instance:
>
> context.h:
> /* The global singleton context aka "g".
>(the name is chosen to be easy to type in a debugger). */
> extern gcc::context *g;
>
> But unfortunately Wshadow=local does not find class members
> shadowed by local variable, which happens for instance in
> wide-int
>
> With -Wshadow, I had to change this in wide-int.h:
>
> Index: gcc/wide-int.h
> ===
> --- gcc/wide-int.h (revision 275545)
> +++ gcc/wide-int.h (working copy)
> @@ -648,9 +648,9 @@ namespace wi
> storage_ref () {}
> storage_ref (const HOST_WIDE_INT *, unsigned int, unsigned int);
>
> -const HOST_WIDE_INT *val;
> -unsigned int len;
> -unsigned int precision;
> +const HOST_WIDE_INT *m_val;
> +unsigned int m_len;
> +unsigned int m_precision;
>
>
> So this change was not necessary for -Wshadow=local, although
> I would think that, shadowing class members is not much different from
> shadowing a local scope.
>
> Not sure if shadowing class members should be part of -Wshadow=local
> instead of -Wshadow=global.
>
That would make sense to me.
>
Re: taking OpenCL C as a built-in lang of GCC?
On Fri, Sep 20, 2019, 1:10 PM Jeff Law wrote: > On 9/12/19 8:48 AM, Jianbin Fang wrote: > > Hello Guys, > > > > > > > > I am working on OpenCL for a couple of years, and would like to ask, > > as for GCC, why not taking OpenCL C as a built-in language in its > > front-end? > There's no inherent reason why we don't support OpenCL C. Someone would > just need to write a suitable front-end for GCC and contribute it. > Or a patch to add it to the C front end. >
Re: Atomics in C++11
On Fri, Sep 20, 2019 at 8:32 AM Nicholas Krause wrote: > I was wondering if its possible to use the C11 atomics library for > multithreading > > GCC. Not sure if its a good idea due to concerns about older plaforms > not having a C11 supported libraries or compiler. I've been wondering if it's time to move to C++11 in general, since we've had compilers with C++11 support for more than 5 years at this point. Jason
Re: Atomics in C++11
On 9/20/19 4:09 PM, Jason Merrill wrote: On Fri, Sep 20, 2019 at 8:32 AM Nicholas Krause wrote: I was wondering if its possible to use the C11 atomics library for multithreading GCC. Not sure if its a good idea due to concerns about older plaforms not having a C11 supported libraries or compiler. I've been wondering if it's time to move to C++11 in general, since we've had compilers with C++11 support for more than 5 years at this point. Jason Jason, It's up to the community at large but both me and the other student who is CCed on this work would find it useful. He wants to use TLS for certain structures and functions to be doubled per thread for no locking e.t.c. and ease of programming. He can add his comments if need be, Nick
Re: Atomics in C++11
On Fri, Sep 20, 2019, 3:12 PM Nicholas Krause wrote: > > On 9/20/19 4:09 PM, Jason Merrill wrote: > > On Fri, Sep 20, 2019 at 8:32 AM Nicholas Krause > wrote: > >> I was wondering if its possible to use the C11 atomics library for > >> multithreading > >> > >> GCC. Not sure if its a good idea due to concerns about older plaforms > >> not having a C11 supported libraries or compiler. > > I've been wondering if it's time to move to C++11 in general, since > > we've had compilers with C++11 support for more than 5 years at this > > point. > > > > Jason > > > Jason, > > It's up to the community at large but both me and the other student who > is CCed > > on this work would find it useful. He wants to use TLS for certain > structures and > > functions to be doubled per thread for no locking e.t.c. and ease of > programming. > When I reviewed TLS support for all RTEMS targets, it wasn't clear that TLS ABI is defined for all supported GCC architectures or that it was universally supported by the backends. I'm not questioning the value of TLS. I'm just wanting to make sure all GCC targets actually support TLS. I may have missed a default generic implementation. It is really hard to find ABI info for all pro processors. I would be thrilled to learn I missed the default generic implementation. This would make supporting it across the 18 architectures RTEMS supports much easier. :) > > > He can add his comments if need be, > > Nick > >
Re: Atomics in C++11
On 9/20/19 4:43 PM, Joel Sherrill wrote: On Fri, Sep 20, 2019, 3:12 PM Nicholas Krause wrote: On 9/20/19 4:09 PM, Jason Merrill wrote: On Fri, Sep 20, 2019 at 8:32 AM Nicholas Krause wrote: I was wondering if its possible to use the C11 atomics library for multithreading GCC. Not sure if its a good idea due to concerns about older plaforms not having a C11 supported libraries or compiler. I've been wondering if it's time to move to C++11 in general, since we've had compilers with C++11 support for more than 5 years at this point. Jason Jason, It's up to the community at large but both me and the other student who is CCed on this work would find it useful. He wants to use TLS for certain structures and functions to be doubled per thread for no locking e.t.c. and ease of programming. When I reviewed TLS support for all RTEMS targets, it wasn't clear that TLS ABI is defined for all supported GCC architectures or that it was universally supported by the backends. I'm not questioning the value of TLS. I'm just wanting to make sure all GCC targets actually support TLS. I may have missed a default generic implementation. It is really hard to find ABI info for all pro processors. I would be thrilled to learn I missed the default generic implementation. This would make supporting it across the 18 architectures RTEMS supports much easier. :) Seems its using IA-64 ABI as reported in a older manual here: https://gcc.gnu.org/onlinedocs/gcc-3.3/gcc/Thread-Local.html Nick He can add his comments if need be, Nick
Re: syncing the GCC vax port, atomic issue
On Fri, Sep 20, 2019 at 11:15:32AM +, co...@sdf.org wrote: > Removed from the diff. Unfortunately this introduces an ICE during the > build of GCC... I took another look at the VAX atomic pattern issue. (http://gnats.netbsd.org/53039) It is a compiler crash to do with the added atomic builtins. The original suggestion was to introduce a reversed pattern, with label / pc swapped. It did not work, unfortunately. The crash backtrace is (gcc-trunk line numbers): during RTL pass: expand /home/fly/gcc/libatomic/flag.c: In function 'atomic_flag_test_and_set': /home/fly/gcc/libatomic/flag.c:36:1: internal compiler error: in patch_jump_insn, at cfgrtl.c:1291 36 | } | ^ 0x718c22 patch_jump_insn /home/fly/gcc/gcc/cfgrtl.c:1290 0x718d2f redirect_branch_edge /home/fly/gcc/gcc/cfgrtl.c:1317 0x71b8c2 rtl_redirect_edge_and_branch /home/fly/gcc/gcc/cfgrtl.c:1450 0x703909 redirect_edge_and_branch(edge_def*, basic_block_def*) /home/fly/gcc/gcc/cfghooks.c:373 0x119ec6e try_forward_edges /home/fly/gcc/gcc/cfgcleanup.c:558 0x11a26ca try_optimize_cfg /home/fly/gcc/gcc/cfgcleanup.c:2961 0x11a26ca cleanup_cfg(int) /home/fly/gcc/gcc/cfgcleanup.c:3175 0x700a41 execute /home/fly/gcc/gcc/cfgexpand.c:6683 This seems to be where GCC has some logic specific to optimizing jumps. Since this isn't a normal jump possible to eliminate by being clever, but rather our only way of doing atomics, my gut feeling is that I should avoid this jump optimizing code entirely. Not telling GCC it's a jump worth optimizing seems to avoid the crash, i.e.: - emit_jump_insn (gen_jbbssi (operands[1], const0_rtx, label, operands[1])); + emit_insn (gen_jbbssi (operands[1], const0_rtx, label, operands[1])); + LABEL_NUSES (label)++; GCC builds, but when building netbsd I get an undefined reference in libstdc++: libstdc++.so: undefined reference to `.L72' libstdc++.so: undefined reference to `.L75' I wonder whether this is a right approach with a slightly off method.
Re: syncing the GCC vax port, atomic issue
On 9/20/19 3:04 PM, co...@sdf.org wrote: > On Fri, Sep 20, 2019 at 11:15:32AM +, co...@sdf.org wrote: >> Removed from the diff. Unfortunately this introduces an ICE during the >> build of GCC... > > I took another look at the VAX atomic pattern issue. > (http://gnats.netbsd.org/53039) > It is a compiler crash to do with the added atomic builtins. > > The original suggestion was to introduce a reversed pattern, with > label / pc swapped. It did not work, unfortunately. > > The crash backtrace is (gcc-trunk line numbers): > during RTL pass: expand > /home/fly/gcc/libatomic/flag.c: In function 'atomic_flag_test_and_set': > /home/fly/gcc/libatomic/flag.c:36:1: internal compiler error: in > patch_jump_insn, at cfgrtl.c:1291 >36 | } > | ^ > 0x718c22 patch_jump_insn > /home/fly/gcc/gcc/cfgrtl.c:1290 > 0x718d2f redirect_branch_edge > /home/fly/gcc/gcc/cfgrtl.c:1317 > 0x71b8c2 rtl_redirect_edge_and_branch > /home/fly/gcc/gcc/cfgrtl.c:1450 > 0x703909 redirect_edge_and_branch(edge_def*, basic_block_def*) > /home/fly/gcc/gcc/cfghooks.c:373 > 0x119ec6e try_forward_edges > /home/fly/gcc/gcc/cfgcleanup.c:558 > 0x11a26ca try_optimize_cfg > /home/fly/gcc/gcc/cfgcleanup.c:2961 > 0x11a26ca cleanup_cfg(int) > /home/fly/gcc/gcc/cfgcleanup.c:3175 > 0x700a41 execute > /home/fly/gcc/gcc/cfgexpand.c:6683 > > > This seems to be where GCC has some logic specific to optimizing jumps. > Since this isn't a normal jump possible to eliminate by being clever, > but rather our only way of doing atomics, my gut feeling is that I > should avoid this jump optimizing code entirely. > > Not telling GCC it's a jump worth optimizing seems to avoid the crash, > i.e.: > > - emit_jump_insn (gen_jbbssi (operands[1], const0_rtx, label, > operands[1])); > + emit_insn (gen_jbbssi (operands[1], const0_rtx, label, operands[1])); > + LABEL_NUSES (label)++; > > GCC builds, but when building netbsd I get an undefined reference in > libstdc++: > libstdc++.so: undefined reference to `.L72' > libstdc++.so: undefined reference to `.L75' > > I wonder whether this is a right approach with a slightly off method. Conditional branching patterns must support the label_ref and pc operands in either position. Everything else I've seen on this thread is just working around that broken aspect of the builtins.md file. (define_insn "jbbssiqi" [(parallel [(set (pc) (if_then_else (ne (zero_extract:SI (match_operand:QI 0 "memory_operand" "g") (const_int 1) (match_operand:SI 1 "general_operand" "nrm")) (const_int 0)) (label_ref (match_operand 2 "" "")) (pc))) (set (zero_extract:SI (match_operand:QI 3 "memory_operand" "+0") (const_int 1) (match_dup 1)) (const_int 1))])] "" "jbssi %1,%0,%l2") Note the position of the (label_ref ...) and (pc) operand. You have to have a pattern where they are reversed as well. As an example look at the branch and branch_reversed patterns in vax.md jeff >
Re: syncing the GCC vax port, atomic issue
On Fri, Sep 20, 2019 at 03:45:46PM -0600, Jeff Law wrote: > Conditional branching patterns must support the label_ref and pc > operands in either position. Everything else I've seen on this thread > is just working around that broken aspect of the builtins.md file. > > > (define_insn "jbbssiqi" > [(parallel > [(set (pc) > (if_then_else > (ne (zero_extract:SI (match_operand:QI 0 "memory_operand" "g") >(const_int 1) >(match_operand:SI 1 "general_operand" "nrm")) > (const_int 0)) > (label_ref (match_operand 2 "" "")) > (pc))) > (set (zero_extract:SI (match_operand:QI 3 "memory_operand" "+0") > (const_int 1) > (match_dup 1)) > (const_int 1))])] > "" > "jbssi %1,%0,%l2") > > Note the position of the (label_ref ...) and (pc) operand. You have to > have a pattern where they are reversed as well. > > As an example look at the branch and branch_reversed patterns in vax.md > > jeff Hi Jeff, Thanks for the advice. Introducing the reversed jbb* patterns doesn't seem to help with the original issue. It crashes building libatomic. I've attempted the following diff, as suggested by Maciej W. Rozycki. (This isn't failing in the latest GCC because it doesn't include vax/builtins.md at all) > Ouch, there are no reversed interlocked branch instructions in the VAX > ISA, so these would have to branch around a jump. ... https://gcc.gnu.org/ml/gcc/2018-04/msg00073.html Is it the intended diff? did I get something wrong? diff --git a/gcc/config/vax/builtins.md b/gcc/config/vax/builtins.md index 2261467..db8ddc40 100644 --- a/gcc/config/vax/builtins.md +++ b/gcc/config/vax/builtins.md @@ -90,6 +90,24 @@ "" "jbssi %1,%0,%l2") +(define_insn "jbbssiqi_reversed" + [(parallel +[(set (pc) + (if_then_else + (ne (zero_extract:SI (match_operand:QI 0 "memory_operand" "g") +(const_int 1) +(match_operand:SI 1 "general_operand" "nrm")) + (const_int 0)) + (pc) + (label_ref (match_operand 2 "" "" + (set (zero_extract:SI (match_operand:QI 3 "memory_operand" "+0") + (const_int 1) + (match_dup 1)) + (const_int 1))])] + "" + "jbssi %1,%0,0f\;jbr %l2\;0:") + + (define_insn "jbbssihi" [(parallel [(set (pc) @@ -107,6 +125,24 @@ "" "jbssi %1,%0,%l2") +(define_insn "jbbssihi_reversed" + [(parallel +[(set (pc) + (if_then_else +(ne (zero_extract:SI (match_operand:HI 0 "memory_operand" "Q") + (const_int 1) + (match_operand:SI 1 "general_operand" "nrm")) +(const_int 0)) +(pc) +(label_ref (match_operand 2 "" "" + (set (zero_extract:SI (match_operand:HI 3 "memory_operand" "+0") + (const_int 1) + (match_dup 1)) + (const_int 1))])] + "" + "jbssi %1,%0,0f\;jbr %l2\;0:") + + (define_insn "jbbssisi" [(parallel [(set (pc) @@ -125,6 +161,25 @@ "jbssi %1,%0,%l2") +(define_insn "*jbbssisi_reversed" + [(parallel +[(set (pc) + (if_then_else + (ne (zero_extract:SI (match_operand:SI 0 "memory_operand" "Q") +(const_int 1) +(match_operand:SI 1 "general_operand" "nrm")) + (const_int 0)) + (pc) + (label_ref (match_operand 2 "" "" + (set (zero_extract:SI (match_operand:SI 3 "memory_operand" "+0") + (const_int 1) + (match_dup 1)) + (const_int 1))])] + "" + "jbssi %1,%0,0f\; jbr %l2\;0:") + + + (define_expand "sync_lock_release" [(set (match_operand:VAXint 0 "memory_operand" "+m") (unspec:VAXint [(match_operand:VAXint 1 "const_int_operand" "n") @@ -162,6 +217,23 @@ "" "jbcci %1,%0,%l2") +(define_insn "jbbcciqi_reversed" + [(parallel +[(set (pc) + (if_then_else + (eq (zero_extract:SI (match_operand:QI 0 "memory_operand" "g") +(const_int 1) +(match_operand:SI 1 "general_operand" "nrm")) + (const_int 0)) + (pc) + (label_ref (match_operand 2 "" "" + (set (zero_extract:SI (match_operand:QI 3 "memory_operand" "+0") + (const_int 1) + (match_dup 1)) + (const_int 0))])] + "" + "jbcci %1,%0,0f\; jbr %l2\;0:") + (define_insn "jbbccihi" [(parallel [(set (pc) @@ -179,6 +251,24 @@ "" "jbcci %1,%0,%l2") + +(define_insn "jbbccihi_reversed" + [(parallel +[(set (pc) + (if_then_else + (eq (zero_extract:SI (match_operand:HI 0 "memory_operand" "Q")
gcc-8-20190920 is now available
Snapshot gcc-8-20190920 is now available on ftp://gcc.gnu.org/pub/gcc/snapshots/8-20190920/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 8 SVN branch with the following options: svn://gcc.gnu.org/svn/gcc/branches/gcc-8-branch revision 276010 You'll find: gcc-8-20190920.tar.xzComplete GCC SHA256=70eb8133b651008e1c69ca4164cc958133bdf5c93d2079bfdb4625fcd0cebf2f SHA1=05e73bbfabedcb6247640bab616c36eacedca8dc Diffs from 8-20190913 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-8 link is updated and a message is sent to the gcc list. Please do not use a snapshot before it has been announced that way.
Re: syncing the GCC vax port, atomic issue
On Fri, Sep 20, 2019 at 10:07:59PM +, co...@sdf.org wrote: > Introducing the reversed jbb* patterns doesn't seem to help with the > original issue. It crashes building libatomic. My loose understanding of what is going on: - GCC emits this atomic in expand. - When cleaning up, it looks for optimizations. - It decides this is a branch to another branch situation, so maybe can be improved. - This fails to output an instruction for unrelated reasons. - Hit an assert. I don't think that we should be trying to combine regular branch + atomic branch in very generic code. My guess is that, if it didn't crash now, it might emit a different kind of branch which loses the atomic qualities, and result in wrong code. I tried to single-step GCC, and it might be trying entirely different instruction patterns. I'm not sure whether I should put a lot of trust in the line numbers shown from .md files, but it's trying nonlocal_goto in vax.md. In any case, nothing from builtins.md.
SSA Trees and Making them MultiThreaded Aware
Richard, Sorry for the second email but I forget in the previous one but Jeff Law at Cauldron stated your the expert for the SSA code. Is it possible to make that code multithreaded in particular the dominator trees. I'm going to start researching that as Giuliano's branch does not build seems he removed the Pthreads flag in the makefile(s). I've already opened a issue with him to fix that. Nick
