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) > > 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. 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? > > 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? > > > -- > Michael Eager > >
