Committed by Rafael Espíndola.
http://llvm-reviews.chandlerc.com/D91
___
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
lgtm
Comment at: lib/Driver/Tools.cpp:5768
@@ -5765,1 +5767,3 @@
+ const bool isAndroid =
+ToolChain.getTriple().getEnvironment() == llvm::Triple::Android;
Nit: you've got the same line broken in a different way in the first chunk
above.
Fixed line break nit. These drive me nuts too!
Hi eugenis,
http://llvm-reviews.chandlerc.com/D91
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D91?vs=246id=250#toc
Files:
lib/Driver/Tools.cpp
test/Driver/clang-translation.c
test/Driver/linux-ld.c
Index:
lgtm2.
Do you have commit access?
On 2 November 2012 08:45, Edwin Vane edwin.v...@intel.com wrote:
Fixed line break nit. These drive me nuts too!
Hi eugenis,
http://llvm-reviews.chandlerc.com/D91
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D91?vs=246id=250#toc
Files:
: Friday, November 02, 2012 2:37 PM
To: reviews+d91+public+6009407b19b79...@llvm-reviews.chandlerc.com
Cc: euge...@google.com; Vane, Edwin; cfe-commits@cs.uiuc.edu
Subject: Re: [cfe-commits] [PATCH] Fix x86 android support in clang
lgtm2.
Do you have commit access?
On 2 November 2012 08:45, Edwin Vane
On 2 November 2012 15:48, Vane, Edwin edwin.v...@intel.com wrote:
No. I have a few revisions in Phab marked as accepted and wasn't sure what to
do next. One revision Chandler indicated as good (D94) except it hasn’t been
accepted yet and I'm stuck on that as well.
I have committed this one.
LGTM with the default cpu test
Comment at: lib/Driver/Tools.cpp:1091
@@ -1090,2 +1090,3 @@
ArgStringList CmdArgs) const {
+ const bool isAndroid = getToolChain().getTriple().getEnvironment() ==
llvm::Triple::Android;
if
Instead of using 'atom' as default cpu name, after some internal
consultation, 'core2' is a better common denominator for x86 android. Added
tests to test/Driver/clang-translation.c.
Note that gcc doesn't appear to do anything special if you don't explicitly
specify which CPU to target
Just want to make sure the new default CPUName is ok.
http://llvm-reviews.chandlerc.com/D91
___
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
On 1 November 2012 15:59, Edwin Vane edwin.v...@intel.com wrote:
Instead of using 'atom' as default cpu name, after some internal
consultation, 'core2' is a better common denominator for x86 android. Added
tests to test/Driver/clang-translation.c.
Why? Can you add a comment saying why
Sorry for the spam. Uploaded the wrong patch file (i.e. the old one again)
last time.
Hi eugenis,
http://llvm-reviews.chandlerc.com/D91
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D91?vs=245id=246#toc
Files:
lib/Driver/Tools.cpp
test/Driver/clang-translation.c
lgtm
On 1 November 2012 16:29, Edwin Vane edwin.v...@intel.com wrote:
Sorry for the spam. Uploaded the wrong patch file (i.e. the old one again)
last time.
Hi eugenis,
http://llvm-reviews.chandlerc.com/D91
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D91?vs=245id=246#toc
Addressed reviewer comments.
Hi eugenis,
http://llvm-reviews.chandlerc.com/D91
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D91?vs=244id=245#toc
Files:
lib/Driver/Tools.cpp
test/Driver/clang-translation.c
test/Driver/linux-ld.c
Index: lib/Driver/Tools.cpp
Hi eugenis,
- -Bsymbolic must be added for x86 as well.
- Default CPU name also set to 'atom' for x86 android.
http://llvm-reviews.chandlerc.com/D91
Files:
lib/Driver/Tools.cpp
test/Driver/linux-ld.c
Index: lib/Driver/Tools.cpp
-CPUName = pentium4;
+CPUName = isAndroid ? atom : pentium4;
This matches gcc behavior? Can you add a test for this part?
Cheers,
Rafael
___
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
On 31 October 2012 15:12, Vane, Edwin edwin.v...@intel.com wrote:
-CPUName = pentium4;
+CPUName = isAndroid ? atom : pentium4;
This matches gcc behavior? Can you add a test for this part?
REV: Sure but what test case would I adjust? These options only seem to show
up for
REV: I know *how* I'd write the test, I'm just not sure which actual test
suite it belongs to. For example, test/Driver/linux-ld.c has linking tests
for android but I couldn't find any existing tests for testing compilation.
If you could point me at the right test suite I'll add tests
On 31 October 2012 15:12, Vane, Edwin edwin.v...@intel.com wrote:
-CPUName = pentium4;
+CPUName = isAndroid ? atom : pentium4;
This matches gcc behavior? Can you add a test for this part?
REV: Sure but what test case would I adjust? These options only seem to show
up for
-CPUName = pentium4;
+CPUName = isAndroid ? atom : pentium4;
This matches gcc behavior? Can you add a test for this part?
REV: Sure but what test case would I adjust? These options only seem to show up
for compiling and I couldn't find a test suite for testing android
19 matches
Mail list logo
