samparker accepted this revision.
samparker added a comment.
This revision is now accepted and ready to land.
> csilUcUsUiUlhfPcPsQcQsQiQlQUcQUsQUiQUlQhQfQPcQPs.
I cry inside every time I see those wonderful strings, but oh well... probably
best to appease the lint gods before committing.
Repo
samparker accepted this revision.
samparker added a comment.
This revision is now accepted and ready to land.
> Eli said in his comments we don't really need to worry about APCS but only to
> linker veneers and I don't think a linker would clear the sticky Q bit or
> touch the GE ones.
Ah, yes,
samparker added a comment.
Thanks for adding the MVE changes, but I also still don't see any DSP tests,
i.e QADD, SADD16.
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5753
+ // ahead and skip over them.
+ if (MI.isKill())
+return outliner::InstrType::Invisible;
-
samparker added inline comments.
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5606
+ // candidates.
+ auto CantGuaranteeValueAcrossCall = [&TRI](outliner::Candidate &C) {
+// If the unsafe registers in this block are all dead, then we don't need
yro
samparker added inline comments.
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5606
+ // candidates.
+ auto CantGuaranteeValueAcrossCall = [&TRI](outliner::Candidate &C) {
+// If the unsafe registers in this block are all dead, then we don't need
yro
samparker added inline comments.
Comment at: llvm/lib/Target/ARM/ARMTargetMachine.cpp:553
addPass(createARMConstantIslandPass());
- addPass(createARMLowOverheadLoopsPass());
+ if (!MachineOutlinerEnabled)
+addPass(createARMLowOverheadLoopsPass());
yroux
samparker added a comment.
Hi Yvan,
Thanks for adding the tests, I've added a few concerns and questions.
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5606
+ // candidates.
+ auto CantGuaranteeValueAcrossCall = [&TRI](outliner::Candidate &C) {
+// If the unsafe r
samparker added a comment.
Hi,
I like reading your code! But I'm concerned around the lack of tests here,
specifically:
- CPSR liveness.
- LR liveness.
- All things PIC related.
- Linkage legality.
- A negative test for Thumb-1.
- Inline asm generally concerns me too.
Commen
samparker added a comment.
How about removing Thumb-1 support until it's properly handled? I also suggest
that this gets broken down a bit too, by handling the different types of
function call in different patches, like getting tail-call support in first or
something. Also, is this currently se
samparker added a comment.
> Even if you only decrease the size you can have a distance increase due to
> aligment constraints
Ah... thanks for raising this. I'll look into it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57054/new/
https://revi
samparker added a comment.
> I can add liveness info into the outlined function, but we will need to do
> the same on AArch64 even if is not necessary.
Hmm, would it be possible to pass a bool to the pass which will control whether
liveness is updated? The other, suboptimal, option would be to
samparker added a comment.
> It might be possible to rearrange Low Overhead Loops to run before
> ConstantIslands, but you'd probably need to do more to make it work properly.
> I don't think ConstantIslands knows how to handle the branches generated by
> LowOverheadLoop.
This, and the fact th
samparker accepted this revision.
samparker added a comment.
This revision is now accepted and ready to land.
LGTM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64471/new/
https://reviews.llvm.org/D64471
___
cfe-commits mailing list
cfe-com
samparker updated this revision to Diff 183820.
samparker added reviewers: phosek, rnk.
samparker added a comment.
Changed the builtins to use W instead of LL. I've also updated the tests,
adding a test for rbitl.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56852/new/
https://reviews
samparker updated this revision to Diff 182481.
samparker added a comment.
Updated wsr, rsr and rbit
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56852/new/
https://reviews.llvm.org/D56852
Files:
include/clang/Basic/BuiltinsAArch64.def
test/CodeGen/arm64-crc32.c
test/CodeGen/bui
samparker created this revision.
samparker added a reviewer: t.p.northover.
Herald added subscribers: kristof.beyls, javed.absar.
The ACLE states that 64-bit crc32 operands are uint64_t so we should have the
clang builtin match this description - which is what we already do for AArch32.
https:/
samparker accepted this revision.
samparker added a comment.
This revision is now accepted and ready to land.
LGTM
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54961/new/
https://reviews.llvm.org/D54961
___
cfe-comm
samparker updated this revision to Diff 161925.
samparker added a comment.
Added test for armv8m.main+dsp.
https://reviews.llvm.org/D51093
Files:
lib/Basic/Targets/ARM.cpp
test/Preprocessor/arm-acle-6.4.c
Index: test/Preprocessor/arm-acle-6.4.c
samparker created this revision.
samparker added reviewers: erichkeane, t.p.northover, SjoerdMeijer.
Herald added a reviewer: javed.absar.
Herald added subscribers: chrib, kristof.beyls.
__ARM_FEATURE_DSP is already set for targets with the +dsp feature. In the
backend, this target feature is als
samparker added a comment.
No tests?
Repository:
rC Clang
https://reviews.llvm.org/D42978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
samparker accepted this revision.
samparker added a comment.
This revision is now accepted and ready to land.
Thanks for the explanation, LGTM, thanks!
https://reviews.llvm.org/D42318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://li
samparker added a comment.
Hi Sjoerd,
Seems sensible to me to treat these two types the same way, though I must admit
having different half types confuses me... So a few questions for my
understanding:
- What issue are you trying to workaround?
-What would the ideal solution be?
- Why do we
samparker accepted this revision.
samparker added a comment.
This revision is now accepted and ready to land.
Great, LGTM, many thanks for doing this!
https://reviews.llvm.org/D40888
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lis
samparker added a comment.
Thanks for looking into this!
Comment at: include/clang/Basic/BuiltinsARM.def:39
BUILTIN(__builtin_arm_qsub, "iii", "nc")
+BUILTIN(__builtin_arm_qdbl, "ii", "nc")
BUILTIN(__builtin_arm_ssat, "iiUi", "nc")
Do we now need a codegen te
samparker updated this revision to Diff 111664.
samparker added a comment.
Reverted the default cpu v8.2-a to generic, I will update
https://reviews.llvm.org/D36667 accordingly. Also fixed up the boolean issues.
https://reviews.llvm.org/D36731
Files:
lib/Driver/ToolChains/Arch/ARM.cpp
test
samparker added a comment.
Thanks guys, I will sort my logic out.
Renato, the new Decode function is really just there to allow me to double
check the target features. This is already possible with AArch64 and it just
felt right to also be able to test ARM in the same way. I've renamed the titl
samparker created this revision.
Herald added subscribers: kristof.beyls, javed.absar, rengolin, aemerson.
This patch introduces support for Cortex-A75 and Cortex-A55, Arm's latest
big.LITTLE A-class cores. They implement the ARMv8.2-A architecture, including
the cryptography and RAS extensions,
27 matches
Mail list logo