saugustine closed this revision.
saugustine added a comment.
Committed as R319464.
https://reviews.llvm.org/D36555
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mgorny resigned from this revision.
mgorny added a comment.
This revision is now accepted and ready to land.
I'm not going to block this but I agree with others that the PPC changes look
like they belong in a separate commit.
https://reviews.llvm.org/D36555
compnerd accepted this revision.
compnerd added a comment.
Would be nice to split up the PPC fixes into its own commit.
https://reviews.llvm.org/D36555
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
mgorny added inline comments.
Comment at: compiler-rt/lib/builtins/CMakeLists.txt:223
+ cpu_model.c
+ divxc3.c
+ fixxfdi.c
saugustine wrote:
> mgorny wrote:
> > This and the following files have only:
> >
> > ```
> > #if !_ARCH_PPC
> > ```
> >
> > so I
weimingz added inline comments.
Comment at: compiler-rt/lib/builtins/CMakeLists.txt:483
+set(powerpc64le_SOURCES ${powerpc64_SOURCES})
+
set(wasm32_SOURCES ${GENERIC_SOURCES})
why these files were not used before? should the change be in a separate patch?
or
weimingz added reviewers: rengolin, compnerd.
weimingz added a comment.
LGTM. Adding Renato and Saleem to approve.
https://reviews.llvm.org/D36555
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
saugustine updated this revision to Diff 112244.
saugustine added a comment.
Remove two files inadvertantly included in last patch.
https://reviews.llvm.org/D36555
Files:
compiler-rt/lib/builtins/CMakeLists.txt
Index: compiler-rt/lib/builtins/CMakeLists.txt
saugustine added a reviewer: weimingz.
saugustine added inline comments.
Comment at: compiler-rt/lib/builtins/CMakeLists.txt:223
+ cpu_model.c
+ divxc3.c
+ fixxfdi.c
mgorny wrote:
> This and the following files have only:
>
> ```
> #if !_ARCH_PPC
> ```
>
>
saugustine updated this revision to Diff 112242.
saugustine added a comment.
Thanks for the various comments. Please take another look.
https://reviews.llvm.org/D36555
Files:
compiler-rt/cmake/Modules/CompilerRTUtils.cmake
compiler-rt/cmake/builtin-config-ix.cmake
saugustine added a comment.
I've cleaned up this patch a bit. Now the only files that are in the x86_ARCH
group are those that require 80 bits floats and cpu_model.c. Tests for all of
these were already disabled on arm and powerpc (because neither has 80-bit
floats), so we knew these library
mgorny requested changes to this revision.
mgorny added a comment.
This revision now requires changes to proceed.
Also, I think that if you're splitting them up, it'd also logical to move them
into a subdirectory, `x86-common` maybe.
Comment at:
aheejin added a comment.
Looks OK to me, but I haven't worked on this part of code either, so I think
someone else should approve it.
https://reviews.llvm.org/D36555
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
nemanjai added a comment.
Thank you for doing this. These were causing warnings with some compilers when
built on PowerPC because the sources were just empty (macro-guarded). Not
compiling them at all is a much cleaner solution.
LGTM but I am far from an authority on this part of the code so
saugustine created this revision.
Herald added a subscriber: mgorny.
https://reviews.llvm.org/D36555
Files:
compiler-rt/lib/builtins/CMakeLists.txt
Index: compiler-rt/lib/builtins/CMakeLists.txt
===
---
14 matches
Mail list logo
