On Thu, Aug 14, 2025, 12:17 PM Gopi Kumar Bulusu <g...@sankhya.com> wrote:
> > namaskaaram > > This patch has been reviewed by Neal Frager - his comments are present in > bugzilla PR 118280 > > If needed I can submit an updated patch with the patch comment updated as > follows > > Reviewed-by: Neal Frager <neal.fra...@amd.com> > > > dhanyavaadaaha > gopi > > On Tue, Jul 22, 2025 at 5:36 PM Gopi Kumar Bulusu <g...@sankhya.com> > wrote: > >> >> >> namaskaaram >> >> Revised patch is attached. This patch no longer changes the default >> option value for >> xl-barrel-shift >> >> On Mon, Jul 14, 2025 at 10:29 AM Gopi Kumar Bulusu <g...@sankhya.com> >> wrote: >> >>> >>> >>> On Fri, Jul 11, 2025 at 8:12 PM Michael Eager <ea...@eagercon.com> >>> wrote: >>> >>>> On 7/10/25 4:41 AM, Gopi Kumar Bulusu wrote: >>>> > >>>> > namaskaaram >>>> >>>> Hi Gopi! >>>> >>>> > >>>> > Please find the patch attached. This addresses regression for >>>> MicroBlaze >>>> > (PR118280) >>>> >>>> Neal Frager posted a different patch (or an RFC) to address pr118280 on >>>> 7/1/25: >>>> https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg376017.html >>>> >>>> The patches are different. Is your patch a replacement for Neal's? >>>> Can you either reconcile the differences or tell me which patch is >>>> correct or better? >>>> >>> >>> This patch overrides the previous one - it works with/without barrel >>> shifter. >>> >>> >>>> >>>> You might also update the PR with this patch and a comment. >>>> pr118280 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118280 >>>> >>> >>> Will do. >>> >>> >>>> > Atomic support enhanced to fix existing atomic_compare_and_swapsi >>>> pattern >>>> > to handle side effects; new patterns atomic_fetch_op and >>>> atomic_test_and_set >>>> > added. As MicroBlaze has no QImode test/set instruction, use shift >>>> magic >>>> > to implement atomic_test_and_set. Make -mxl-barrel-shift the default >>>> to keep >>>> > the default atomics code tight. >>>> >>>> Include the PR which is resolved in the patch comments. >>>> >>> >>> It is already there (Subject) >>> >> >> Added PR in comments. >> >> >>> >>> >>>> >>>> I'm not quite sure what you mean by "keep the default atomics code >>>> tight". >>>> >>>> Neal suggested making -mxl-barrel-shift default for linux, but not >>>> default for bare-metal. This might be better for backward >>>> compatibility, but depends on whether there are any MB configurations >>>> which do not include a barrel shifter. If someone does have a MB config >>>> without a barrel shifter and tries to recompile after this patch, it's >>>> possible that invalid code would be silently generated. >>>> >>> >>> I will address this and submit a revision-2 patch. >>> >> >> With this patch revision there are no changes to default value of >> xl-barrel-shift >> >> >>> >>> dhanyavaadaaha >>> gopi >>> >>> >>>> >>>> > >>>> > Files Changed >>>> > >>>> > * gcc/config/microblaze/iterators.md: New >>>> > * microblaze-protos.h/microblaze.cc : Add microblaze_subword_address >>>> > * gcc/config/microblaze/microblaze.md: constants: Add >>>> UNSPECV_CAS_BOOL, >>>> > UNSPECV_CAS_MEM, UNSPECV_CAS_VAL, UNSPECV_ATOMIC_FETCH_OP >>>> > type: add atomic >>>> > * gcc/config/microblaze/microblaze.h: TARGET_DEFAULT : Add >>>> MASK_BARREL_SHIFT >>>> > * gcc/config/microblaze/sync.md: Add atomic_fetch_<atomic_optab>si >>>> > atomic_test_and_set >>>> > >>>> > Target Checked >>>> > microblazeel-amd-linux >>>> > >>>> > Testing >>>> > >>>> > deja-g++ >>>> > >>>> > === g++ Summary === >>>> > >>>> > # of expected passes 237906 >>>> > # of unexpected failures 4165 >>>> > # of unexpected successes 3 >>>> > # of expected failures 2180 >>>> > # of unresolved testcases 645 >>>> > # of unsupported tests 2658 >>>> > >>>> > >>>> > deja-libstdcpp >>>> > >>>> > === libstdc++ Summary === >>>> > >>>> > # of expected passes 18180 >>>> > # of unexpected failures 311 >>>> > # of expected failures 133 >>>> > # of unresolved testcases 18 >>>> > # of unsupported tests 853 >>>> > >>>> > Includes Test case 29_atomics/atomic_flag/clear/1.cc (which checks >>>> for >>>> > atomic_test_and_set) >>>> >>>> I don't have a baseline to compare these test results with, or test >>>> results before applying this patch, so the results don't have any >>>> meaning to me. Was the test case 29 failing before applying the patch >>>> and succeeding after? Were there any other differences? >>>> >>> >> The test case did not build and fails execution without the patch. >> >> patch-v2 yields exactly the same results for libstdc++ tests as above. >> >> >>> >>>> Neal indicated that patch was tested using buildroot with >>>> target=microblazeel-buildroot-linux-gnu. It looks like you have >>>> target=microblazeel-amd-linux. How are you running the test suite? >>>> >>> >> Using QEMU from peta-linux image. >> >> dhanyavaadaaha >> gopi >> >> >>> >>>> >>>> -- >>>> Michael Eager >>>> >>>>
