Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: NDK27 clang hangs compiling google/skia #2059

Closed
tiagomacarios opened this issue Aug 22, 2024 · 7 comments
Closed

[Bug]: NDK27 clang hangs compiling google/skia #2059

tiagomacarios opened this issue Aug 22, 2024 · 7 comments
Assignees
Labels

Comments

@tiagomacarios
Copy link

Description

Clang hangs indefinitely when compiling version m101 of google/skia.
Repro attached - skia_hang.zip

I tried to repro the issue with clang version 18.1.0rc and it did not repro.

PS: Would it be possible to add pdbs to the release? This would help us try to find workarounds and/or provide you more info about the issue.

Upstream bug

No response

Commit to cherry-pick

No response

Affected versions

r27

Canary version

No response

Host OS

Windows

Host OS version

Windows 10/11

Affected ABIs

arm64-v8a

@DanAlbert

This comment was marked as off-topic.

@github-project-automation github-project-automation bot moved this to Unconfirmed in NDK r27b Aug 22, 2024
@github-project-automation github-project-automation bot moved this to Unconfirmed in NDK r28 Aug 22, 2024
@DanAlbert DanAlbert moved this from Unconfirmed to Triaged in NDK r27b Aug 22, 2024
@DanAlbert
Copy link
Member

This probably won't make it into r27b btw, but I'm triaging it there for now in case we have a fix before that goes to QA (supposed to have happened already but we're blocked on some infra issues, so maybe). If not, we'll aim for r27c.

I don't suppose you know if this is a regression from r26? We'll try to find out ourselves, but if you already know that saves us some time.

@tiagomacarios
Copy link
Author

I don't suppose you know if this is a regression from r26? We'll try to find out ourselves, but if you already know that saves us some time.

We are currently on r26b and it compiles fine.

@DanAlbert
Copy link
Member

Thanks for confirming 👍

@pirama-arumuga-nainar
Copy link
Collaborator

I can reproduce the timeout. It doesn't reproduce with clang-r530567 (slated tentatively for r28 - in case @DanAlbert was wondering). Bisecting to find the fix.

Given the blockers in generating new clang prebuilts for r27b, we could potentially include the fix in r27b.

@DanAlbert DanAlbert removed this from NDK r28 Aug 23, 2024
@pirama-arumuga-nainar
Copy link
Collaborator

070848c17c2944afa494d42d3ad42929f3379842 is the first new commit            
commit 070848c17c2944afa494d42d3ad42929f3379842                                                          
Author: Nikita Popov <npopov@redhat.com>                                                                                                                                                                           
Date:   Tue Feb 13 09:29:56 2024 +0100                                                                   
                                                                                                         
    [AArch64][GISel] Don't pointlessly lower G_TRUNC (#81479)        
                                             
    If we have something like G_TRUNC from v2s32 to v2s16, then lowering
    this to a concat of two G_TRUNC s32 to s16 followed by G_TRUNC from
    v2s16 to v2s8 does not bring us any closer to legality. In fact, the
    first part of that is a G_BUILD_VECTOR whose legalization will produce a                                                                                                                                       
    new G_TRUNC from v2s32 to v2s16, and both G_TRUNCs will then get
    combined to the original, causing a legalization cycle. 
                                                                                                         
    Make the lowering condition more precise, by requiring that the original              
    vector is >128 bits, which is I believe the only case where this
    specific splitting approach is useful.
     
    Note that this doesn't actually produce a legal result (the alwaysLegal
    is a lie, as before), but it will cause a proper globalisel abort
    instead of an infinite legalization loop.
     
    Fixes https://github.com/llvm/llvm-project/issues/81244.

 .../Target/AArch64/GISel/AArch64LegalizerInfo.cpp  |  5 ++---
 .../CodeGen/AArch64/GlobalISel/legalize-xtn.mir    | 24 ++++++++++++++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)

@pirama-arumuga-nainar
Copy link
Collaborator

r.android.com/3238718 cherry-picks the fix. Uploading new prebuilts shortly.

@DanAlbert DanAlbert moved this from Triaged to Needs prebuilt update in NDK r27b Aug 27, 2024
@DanAlbert DanAlbert moved this from Needs prebuilt update to Merged in NDK r27b Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Merged
Development

No branches or pull requests

4 participants