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

Chromium aarch64 builds fail with "Cannot select: t33: f64 = AArch64ISD::FMOV Constant:i32<0>" #35717

Closed
zmodem opened this issue Feb 13, 2018 · 14 comments
Labels
bugzilla Issues migrated from bugzilla

Comments

@zmodem
Copy link
Collaborator

zmodem commented Feb 13, 2018

Bugzilla Link 36369
Resolution FIXED
Resolved on Feb 20, 2018 00:53
Version trunk
OS Linux

Extended Description

See https://bugs.chromium.org/p/chromium/issues/detail?id=811767#c1 for reproducer.

@zmodem
Copy link
Collaborator Author

zmodem commented Feb 13, 2018

Bisection points to this one:


Author: evandro
Date: Mon Feb 12 08:41:41 2018
New Revision: 324903

URL: http://llvm.org/viewvc/llvm-project?rev=324903&view=rev
Log:
[AArch64] Refactor identification of SIMD immediates

Get rid of icky goto loops and make the code easier to maintain (NFC).

Differential revision: https://reviews.llvm.org/D42723

Modified:
llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp

@zmodem
Copy link
Collaborator Author

zmodem commented Feb 13, 2018

Reverted in r325034 in the meantime

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 13, 2018

Would it be possible to attach a preprocessed version of the offending code, please?

@zmodem
Copy link
Collaborator Author

zmodem commented Feb 14, 2018

Would it be possible to attach a preprocessed version of the offending code,
please?

It was too large to attach here directly, so I added it to
https://bugs.chromium.org/p/chromium/issues/detail?id=811767#c1 which I linked to above.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 15, 2018

That file doesn't seem to be the preprocessed output that results when the -E option is specified. I'm still getting too many compilation errors and cannot confirm the fix for it. Please, attach the .i file to the Chromium bug as well.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 16, 2018

Posted fix for review at https://reviews.llvm.org/D42133

@zmodem
Copy link
Collaborator Author

zmodem commented Feb 16, 2018

preprocessed repro

That file doesn't seem to be the preprocessed output that results when the
-E option is specified. I'm still getting too many compilation errors and
cannot confirm the fix for it. Please, attach the .i file to the Chromium
bug as well.

It's the reproducer created by clang when it crashed. I think that means it's got all the #include files pulled in, but it still has other preprocessor directives.

Attaching a preprocessed version which shows the error when invoked as:

$ clang -cc1 -triple aarch64--linux-android -emit-obj -target-cpu generic -target-feature +neon -target-abi aapcs -Oz -std=gnu++14 -vectorize-slp -x c++ /tmp/a.ii

@zmodem
Copy link
Collaborator Author

zmodem commented Feb 16, 2018

Posted fix for review at https://reviews.llvm.org/D42133

I applied this on top of r324903 but still get the same error using the repro in #c7.

@zmodem
Copy link
Collaborator Author

zmodem commented Feb 16, 2018

Posted fix for review at https://reviews.llvm.org/D42133

I applied this on top of r324903 but still get the same error using the
repro in #c7.

Oh, or did you mean https://reviews.llvm.org/D43364 ?

I applied this too, but still get the same failure.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 16, 2018

Got it narrowed down to:

define void @​v2i32st(<2 x i32>* %p) nounwind {
store <2 x i32> <i32 0, i32 1073741824>, <2 x i32>* %p, align 8
ret void
}

Thank you!

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 16, 2018

Please, confirm solution in https://reviews.llvm.org/D43364.

Thank you.

@zmodem
Copy link
Collaborator Author

zmodem commented Feb 19, 2018

Please, confirm solution in https://reviews.llvm.org/D43364.

Yes, that seems to work.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 19, 2018

I'd appreciate if you could review the patch too. Thanks for confirming this bug fixed.

@zmodem
Copy link
Collaborator Author

zmodem commented Feb 20, 2018

I'd appreciate if you could review the patch too. Thanks for confirming
this bug fixed.

Sorry, I don't know anything about aarch64 isel, so I'm not a good reviewer for this.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

2 participants