peter.smith added a comment.
Added LLVM_FALLTHROUGH; in r344890.
Repository:
rC Clang
https://reviews.llvm.org/D52784
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
xbolva00 added inline comments.
Comment at: lib/Driver/ToolChains/Gnu.cpp:241
+ case llvm::Triple::thumbeb:
+IsBigEndian = true;
+ case llvm::Triple::arm:
peter.smith wrote:
> xbolva00 wrote:
> > Gnu.cpp:241:17: warning: this statement may fall through
> >
peter.smith added a comment.
Thanks for pointing that out, I'm out of office today, will look at describing
the intention to fall through when I get back in on Monday.
Comment at: lib/Driver/ToolChains/Gnu.cpp:241
+ case llvm::Triple::thumbeb:
+IsBigEndian = true;
+ case
xbolva00 added inline comments.
Comment at: lib/Driver/ToolChains/Gnu.cpp:241
+ case llvm::Triple::thumbeb:
+IsBigEndian = true;
+ case llvm::Triple::arm:
Gnu.cpp:241:17: warning: this statement may fall through
[-Wimplicit-fallthrough=]
IsBigEndian =
nickdesaulniers added a comment.
Thanks again for this patch!
Repository:
rC Clang
https://reviews.llvm.org/D52784
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC344597: [ARM][AArch64] Pass through endian flags to
assembler and linker. (authored by psmith, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52784?vs=169689&id=169799#toc
Repositor
nickdesaulniers accepted this revision.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.
Thanks for this patch. With it I was able to link+boot a BE aarch64 Linux
kernel (and a LE aarch64 Linux kernel).
Comment at: lib/Driver/ToolChains/Gnu.cp
peter.smith updated this revision to Diff 169689.
peter.smith marked 3 inline comments as done.
peter.smith added a comment.
Updated diff to reflect review comments. Main changes are:
- isArmBigEndian always returns false if the target architecture isn't Arm.
- Added tests to make sure "--be8" do
peter.smith marked 7 inline comments as done.
peter.smith added a comment.
Thanks very much for the comments. I'll post an update shortly.
Comment at: lib/Driver/ToolChains/Gnu.cpp:357-364
+const char* EndianFlag = "-EL";
+if (isArmBigEndian(Triple, Args)) {
+ Endi
nickdesaulniers added inline comments.
Comment at: lib/Driver/ToolChains/Gnu.cpp:357-364
+const char* EndianFlag = "-EL";
+if (isArmBigEndian(Triple, Args)) {
+ EndianFlag = "-EB";
+ arm::appendBE8LinkFlag(Args, CmdArgs, Triple);
+}
+else if (Arch == llv
nickdesaulniers added a comment.
Thanks for the addition of the flags to the linker. Interesting note about
`-m*-endian` only being applicable for armv7. Just some minor nits left.
With the current version of the patch, I can now assemble, link, and boot in a
virtualized environment a big-endi
peter.smith updated this revision to Diff 169386.
peter.smith added a comment.
I've decided to roll the linker changes in with the assembler ones as the
linker use case affects the design. It turns out that only Arm needs to check
to see if the -mbig-endian and -mlittle-endian flags override the
peter.smith added a comment.
On reflection after looking at what would be needed for the linker
tools::gnutools::Linker::ConstructJob() I think it may be better to move the
triple detection into a separate function. I'll work on that and will hopefully
post an update soon.
https://reviews.llv
peter.smith added a comment.
Thanks for the comments. I agree with you that the -EB and -EL flags need to be
passed to the linker as well, I kind of expected that ld.bfd would infer it
from the endianness of the first object but it doesn't seem to do that. If it's
ok I'll do that in a separate
nickdesaulniers added inline comments.
Comment at: lib/Driver/ToolChains/Gnu.cpp:543-549
+static const char* GetEndianArg(bool IsBigEndian, const ArgList &Args) {
+ if (Arg *A = Args.getLastArg(options::OPT_mlittle_endian,
+ options::OPT_mbig_endian
nickdesaulniers added a comment.
I think we should pass these flags along to the linker, too. Not just the
assembler. Maybe within tools::gnutools::Linker::ConstructJob() within
lib/Driver/ToolChains/Gnu.cpp?
https://reviews.llvm.org/D52784
___
c
peter.smith added a comment.
Ping. Does anyone have any changes they'd like me to make?
https://reviews.llvm.org/D52784
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
peter.smith updated this revision to Diff 167955.
peter.smith added a comment.
Updated diff to add more tests, including some that use -mlittle-endian when
the target is big-endian.
https://reviews.llvm.org/D52784
Files:
lib/Driver/ToolChains/Gnu.cpp
test/Driver/linux-as.c
Index: test/Dri
peter.smith added a comment.
In https://reviews.llvm.org/D52784#1252569, @rengolin wrote:
> Hi Peter,
>
> Looks ok. Why did you need to change all cmdlines to have -EL? I imagine you
> just need one for each case, everything else remains the default (which
> should still work).
I thought that
rengolin added a comment.
Hi Peter,
Looks ok. Why did you need to change all cmdlines to have -EL? I imagine you
just need one for each case, everything else remains the default (which should
still work).
Also, it would be interesting to know what happens on cases like "aarch64_be
-EL" et al,
peter.smith created this revision.
peter.smith added reviewers: rengolin, compnerd, olista01.
Herald added subscribers: chrib, kristof.beyls, srhines.
Herald added a reviewer: javed.absar.
The big-endian arm32 Linux builds are currently failing when the -mbig-endian
and -fno-use-integrated-as fla
21 matches
Mail list logo
