Mostly copy-and-paste from existing Sparc architecture.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
In the description, you meant to say copied from Sparc V8, right? (as this has created a sparcv8 variant, not a v9 variant).
(and of course some test cases)
lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.cpp | ||
---|---|---|
54 ↗ | (On Diff #22990) | Why a new copy of this? Shouldn't it just use createSparcMCAsmInfo? |
lib/Target/Sparc/SparcTargetMachine.cpp | ||
29 ↗ | (On Diff #22990) | Perhaps less confusing to just have a bool isLittleEndian instead of a char passed in here? |
I realize this is going to seem like a terrible nit to pick on this patch, but I'd really prefer not to have the _ in sparc_le... none of the rest of the explicit little endian triples use an underscore.
A few additional comments inline. I'll want to take another look when you've fixed these if you don't mind.
Thanks!
-eric
lib/Target/Sparc/SparcSubtarget.cpp | ||
---|---|---|
50 ↗ | (On Diff #23000) | Two things here: a) Comments are sentences |
lib/Target/Sparc/SparcTargetMachine.cpp | ||
66 ↗ | (On Diff #23000) | If you're using sparc*le as your triple (e.g. sparcle, sparcv8le, sparcv9le) you don't need to pass isLittleEndian down to things, you can just use the target triple string and check for the architecture. |
Can you split out the triple part of the patch and add a test for it in unittests please? It'll reduce the size of the patch here a bit. You'll also want to clang-format this patch.
-eric
factored out the changes to Triple as requested - http://reviews.llvm.org/D9263
lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.cpp | ||
---|---|---|
54 ↗ | (On Diff #22990) | Done |
lib/Target/Sparc/SparcTargetMachine.cpp | ||
29 ↗ | (On Diff #22990) | Done |
will upload new patch with clang-format-diff. also explained the use of temp vars for '=='
lib/Target/Sparc/MCTargetDesc/SparcAsmBackend.cpp | ||
---|---|---|
224 ↗ | (On Diff #24489) | It was cargo-cultism based on the example above. |