Not an issue, but somewhere to post test results. #1
Comments
Hi all, We noticed that calls to the vadcq and vsbcq intrinsics, both of which use __builtin_arm_set_fpscr_nzcvqc to set the Carry flag in the FPSCR, would produce the following code: ``` < r2 is the *carry input > vmrs r3, FPSCR_nzcvqc bic r3, r3, #536870912 orr r3, r3, r2, lsl #29 vmsr FPSCR_nzcvqc, r3 ``` when the MVE ACLE instead gives a different instruction sequence of: ``` < Rt is the *carry input > VMRS Rs,FPSCR_nzcvqc BFI Rs,Rt,#29,#1 VMSR FPSCR_nzcvqc,Rs ``` the bic + orr pair is slower and it's also wrong, because, if the *carry input is greater than 1, then we risk overwriting the top two bits of the FPSCR register (the N and Z flags). This turned out to be a problem in the header file and the solution was to simply add a `& 1x0u` to the `*carry` input: then the compiler knows that we only care about the lowest bit and can optimise to a BFI. Ok for trunk? Thanks, Stam Markianos-Wright gcc/ChangeLog: * config/arm/arm_mve.h (__arm_vadcq_s32): Fix arithmetic. (__arm_vadcq_u32): Likewise. (__arm_vadcq_m_s32): Likewise. (__arm_vadcq_m_u32): Likewise. (__arm_vsbcq_s32): Likewise. (__arm_vsbcq_u32): Likewise. (__arm_vsbcq_m_s32): Likewise. (__arm_vsbcq_m_u32): Likewise. * config/arm/mve.md (get_fpscr_nzcvqc): Make unspec_volatile. gcc/testsuite/ChangeLog: * gcc.target/arm/mve/mve_vadcq_vsbcq_fpscr_overwrite.c: New.
|
I would like to give it a try, but I don't know what commits should I add to the Homebrew patches, the only recent commit I can find is a README update. |
9fd551b is the relevant commit. Hmmmm.. let me check this, something appears to have gone wrong. |
|
hmm now I am confused - that patch is in the 13.3-darwin-r0 release .. so it should be (AFAIK) in the Homebrew kit - @fxcoudert ? [so the r1 is actually unnecessary :( .. ] please could you try just building the 13.3-darwin-r{0,1} branch and check if your issue is resolved - I cannot realistically check more than the trees I produce. |
Perhaps I'm missing something, but this commit seems to change only some comments: |
correct - because the patch had already been applied as : 8543cc2 So that all the reapplication did was to update the generation date info. So - 13.3-darwin-r0 contains this fix; (as does 13.3-darwin-r1) therefore, please build one of those trees and confirm (or not) that your problem is fixed. |
The gcc@13.rb file includes a link to: I searched for |
|
@ilg-ul the fix is not in Homebrew so far. Homebrew builds binaries for each macOS version (and matching SDK) separately, so the issue is not as problematic for us as it can be for a single binary distribution targeting multiple macOS versions. Iain has linked the commit that you need to apply. |
|
So the Homebrew patch is not based on the |
"based on" != "identical" I think - HB omits the testsuite changes, for example. |
It is based on the pre-release, which is almost the same, but did not include the fixincludes removal. I plan to include that eventually, and am testing it on GCC 14 first: Homebrew/homebrew-core#173647 And yes, based on != identical. Each distributor (Homebrew like others) makes choices for distribution, based on their needs and constraints. |
|
I see, I assumed that the HB patches use exactly the commits tagged by Iain, like I prepared a patch like this: git diff origin/gcc-13-3..fa196a8618c62428a372fb251f9fa292d4f275c2(the numeric commit sha will eventually become Apart from the testsuite commits which are not relevant to me, are there any other commits that you exclude when preparing the patch for the HB distribution? I'm trying to identify a way to create these patches in order to remain functionally compatible with the HB releases. |
|
With the updated patch the problem related to the availability macros seems fixed. There is only one test failing in certain conditions, both for 14.a and 13.3, I'll prepare a separate issue for it. |
No description provided.
The text was updated successfully, but these errors were encountered: