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 >>> >>>
