- User Since
- Apr 13 2018, 4:23 PM (61 w, 2 d)
Thanks for the patch and following up on the review comments. Let's work on getting you commit access now.
Thanks for taking a look @MaskRay ! From this thread from our mailing list, I think that the different entity size and alignment may be a bug: https://groups.google.com/forum/#!topic/clang-built-linux/0V6APpBcl3k
Fri, Jun 14
https://llvm.org/docs/TestingGuide.html mentions .cfg files, but tests I've looked at don't seem to have a .cfg in their directory.
There are lots of tests under clang/test/ that target arm-- as a -triple. How is it that they don't fail in the same way? %clang_cc1 -triple arm-- is somehow different?
Thu, Jun 13
but as part of this commit
Thanks for the patch, I look forward to this feature!
Tue, Jun 11
Fri, Jun 7
Looks like this is not the correct solution. See also: https://android-review.googlesource.com/c/toolchain/llvm-project/+/979791
Wed, Jun 5
For future travelers, we're tracking 2 related bugs:
- -no-integrated-as providing different results: https://github.com/ClangBuiltLinux/linux/issues/500
- IfConverter asserting on INLINEASM_BR MachineInstrs: https://github.com/ClangBuiltLinux/linux/issues/494
Tue, Jun 4
parting thoughts @eli.friedman ?
Sun, Jun 2
- implement @RKSimon's suggestions
Fri, May 31
- git-clang-format HEAD~
- prefer assertion to release build checks
- use @MaskRay's trick
Also, I'm fine with these as by-inspection changes, but if there are reasonably-easy ways to write test cases (or update existing ones), please do.
Adding @hfinkel for review because I think we can go further. The only other override of TargetInstrInfo::isUnpredicatedTerminator is PPCInstrInfo::isUnpredicatedTerminator and it almost matches, modulo a check for whether the MachineInstr isPredicable. I'm not sure if that's intentional, or bug, but adding return TargetInstrInfo::isUnpredicatedTerminator(MI); as the first statement in PPCInstrInfo::isUnpredicatedTerminator doesn't cause any failures in existing tests.
- add fixme
Thu, May 30
- fix typo
It's possible the transform isn't profitable if we're optimizing for size, but that's a completely different issue.
- get rid of some debug info
- hoist INLINEASM_BR check into shouldTailDuplicate
so probably a bug in if conversion. Maybe the same issue as https://bugs.llvm.org/show_bug.cgi?id=41121 .
Was this committed accidentally?
Wed, May 29
Otherwise, conservatively do not allow INLINEASM_BR to be predicable. (I'm leaning more towards this, since such machinery would be rewriting the inline asm the user specified).
Could this be fixed in the if conversion pass IfConverter::ScanInstructions() by having TargetInstrInfo::isPredicable(MachineInstr&) reject TargetOpcode::INLINEASM_BR?
Tue, May 28
- get rid of some debug info
Fri, May 24
Ok, from the Linux kernel's perspective, I believe we have worked out all underlying issues in LLVM wrt callbr that would prevent us from using asm goto successfully. This patch now has my blessing; thanks for all the hard work that went into this large feature. Please wait for a final LGTM from @rsmith .
Landing for now to unblock asm goto https://reviews.llvm.org/D56571. I will try to get a test in for this later.
constant pool goes at the end by default
Tue, May 21
Also of note is that no existing test covers this case.
because is it a string reference? (that might be invalidated by the move), a return by value (that then means it's reference extending, which is best avoided unless really needed),
Wouldn't just deleting L142 from the existing sources be simpler? (or moving L146-148 into the for loop at the end (L142)?
Correction: -DLLVM_BUILD_LLVM_DYLIB="ON" is required to run ninja check-llvm-bugpoint.
- capitalize LLVM