[PATCH] D38168: [mips] Accept but ignore -m(no-)branch-likely

[Simon Dardis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL314213: [mips] Accept but ignore -m(no-)branch-likely 
(authored by sdardis).

Repository:
  rL LLVM

https://reviews.llvm.org/D38168

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/test/Driver/mips-features.c


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -2047,6 +2047,10 @@
 def mno_check_zero_division : Flag<["-"], "mno-check-zero-division">,
   Group;
 def mcompact_branches_EQ : Joined<["-"], "mcompact-branches=">, Group;
+def mbranch_likely : Flag<["-"], "mbranch-likely">, Group,
+  IgnoredGCCCompat;
+def mno_branch_likely : Flag<["-"], "mno-branch-likely">, Group,
+  IgnoredGCCCompat;
 def mdsp : Flag<["-"], "mdsp">, Group;
 def mno_dsp : Flag<["-"], "mno-dsp">, Group;
 def mdspr2 : Flag<["-"], "mdspr2">, Group;
Index: cfe/trunk/test/Driver/mips-features.c
===
--- cfe/trunk/test/Driver/mips-features.c
+++ cfe/trunk/test/Driver/mips-features.c
@@ -392,3 +392,13 @@
 // LONG-CALLS-ON: "-target-feature" "+long-calls"
 // LONG-CALLS-OFF: "-target-feature" "-long-calls"
 // LONG-CALLS-DEF-NOT: "long-calls"
+//
+// -mbranch-likely
+// RUN: %clang -target -mips-mti-linux-gnu -### -c %s -mbranch-likely 2>&1 \
+// RUN:   | FileCheck --check-prefix=BRANCH-LIKELY %s
+// BRANCH-LIKELY: argument unused during compilation: '-mbranch-likely'
+//
+// -mno-branch-likely
+// RUN: %clang -target -mips-mti-linux-gnu -### -c %s -mno-branch-likely 2>&1 \
+// RUN:   | FileCheck --check-prefix=NO-BRANCH-LIKELY %s
+// NO-BRANCH-LIKELY: argument unused during compilation: '-mno-branch-likely'


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -2047,6 +2047,10 @@
 def mno_check_zero_division : Flag<["-"], "mno-check-zero-division">,
   Group;
 def mcompact_branches_EQ : Joined<["-"], "mcompact-branches=">, Group;
+def mbranch_likely : Flag<["-"], "mbranch-likely">, Group,
+  IgnoredGCCCompat;
+def mno_branch_likely : Flag<["-"], "mno-branch-likely">, Group,
+  IgnoredGCCCompat;
 def mdsp : Flag<["-"], "mdsp">, Group;
 def mno_dsp : Flag<["-"], "mno-dsp">, Group;
 def mdspr2 : Flag<["-"], "mdspr2">, Group;
Index: cfe/trunk/test/Driver/mips-features.c
===
--- cfe/trunk/test/Driver/mips-features.c
+++ cfe/trunk/test/Driver/mips-features.c
@@ -392,3 +392,13 @@
 // LONG-CALLS-ON: "-target-feature" "+long-calls"
 // LONG-CALLS-OFF: "-target-feature" "-long-calls"
 // LONG-CALLS-DEF-NOT: "long-calls"
+//
+// -mbranch-likely
+// RUN: %clang -target -mips-mti-linux-gnu -### -c %s -mbranch-likely 2>&1 \
+// RUN:   | FileCheck --check-prefix=BRANCH-LIKELY %s
+// BRANCH-LIKELY: argument unused during compilation: '-mbranch-likely'
+//
+// -mno-branch-likely
+// RUN: %clang -target -mips-mti-linux-gnu -### -c %s -mno-branch-likely 2>&1 \
+// RUN:   | FileCheck --check-prefix=NO-BRANCH-LIKELY %s
+// NO-BRANCH-LIKELY: argument unused during compilation: '-mno-branch-likely'
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis edited subscribers, added: cfe-commits; removed: llvm-commits.
sdardis added a comment.

+CC cfe-commits, -CC llvm-commits.

Some comments inlined.

I've managed to run the test-suite on one of my machines here and I'm seeing 3 
failures:

  libunwind :: libunwind_01.pass.cpp
  libunwind :: libunwind_02.pass.cpp
  libunwind :: unw_getcontext.pass.cpp

This is with a GCC toolchain, 6.3. Did you test with clang?




Comment at: include/__libunwind_config.h:54
 #  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 32
+# elif defined(__mips_o32)
+#  define _LIBUNWIND_TARGET_MIPS_O32 1

Can you avoid using the __mips_o32 macro and instead use defined(__mips__) && 
_MIPS_SIM == _ABIO32 ?

It appears the __mips_o32 is a FreeBSD specific macro. Also, test for 
__mips_soft_float .



Comment at: include/__libunwind_config.h:59
+#  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 32
+# elif defined(__mips_n64)
+#  define _LIBUNWIND_TARGET_MIPS_N64 1

Likewise "defined(__mips__) && _MIPS_SIM == _ABI64 && __mips_soft_float"



Comment at: src/UnwindRegistersRestore.S:492
 
+#elif defined(__mips_o32)
+

defined(__mips_o32) -> defined(__mips__) && _MIPS_SIM == _ABIO32



Comment at: src/UnwindRegistersRestore.S:500
+//
+  .set noreorder
+  .set noat

Prefix this with:

   .set push
   .set nomacro

Also, the assembler directives should come after the function declaration / 
defines.



Comment at: src/UnwindRegistersRestore.S:539
+  lw$4, (4 * 4)($4)
+
+#elif defined(__mips_n64)

   .set pop



Comment at: src/UnwindRegistersRestore.S:540
+
+#elif defined(__mips_n64)
+

defined(__mips__) && _MIPS_SIM == _ABI64



Comment at: src/UnwindRegistersRestore.S:550
+  .set noat
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind18Registers_mips_n646jumptoEv)
+  // r0 is zero

See my comments on the o32 assembly.



Comment at: src/UnwindRegistersSave.S:90
 
+#elif defined(__mips_o32)
+

See my previous comments about this preprocessor macro.



Comment at: src/UnwindRegistersSave.S:101
+DEFINE_LIBUNWIND_FUNCTION(unw_getcontext)
+  .set noreorder
+  # s0 - s7

Wrap this with

   .set push
   .set noat
   .set nomacro



Comment at: src/UnwindRegistersSave.S:120
+  sw$30, (4 * 30)($4)
+
+#elif defined(__mips_n64)

and:
   .set pop



Comment at: src/UnwindRegistersSave.S:121
+
+#elif defined(__mips_n64)
+

Again, see my previous comments on this preprocessor define.



Comment at: src/UnwindRegistersSave.S:132
+DEFINE_LIBUNWIND_FUNCTION(unw_getcontext)
+  .set noreorder
+  # s0 - s7

Wrap this with

   .set push
   .set noat
   .set nomacro



Comment at: src/UnwindRegistersSave.S:151
+  sd$30, (8 * 30)($4)
+
 # elif defined(__mips__)

   .set pop



Comment at: src/libunwind.cpp:66
 #elif defined(__mips__)
 # warning The MIPS architecture is not supported.
 #else

The MIPS architecture is not supported with this ABI and environment!


https://reviews.llvm.org/D38110



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Thanks for the pointer to that patch, I'll take a look tomorrow.




Comment at: src/UnwindRegistersRestore.S:492
 
+#elif defined(__mips__) && _MIPS_SIM == _ABIO32
+

Needs checking for soft-float.



Comment at: src/UnwindRegistersRestore.S:543
+
+#elif defined(__mips__) && _MIPS_SIM == _ABI64
+

Needs checking for soft-float.



Comment at: src/UnwindRegistersSave.S:90
 
+#elif defined(__mips__) && _MIPS_SIM == _ABIO32
+

Needs checking for soft-float.



Comment at: src/UnwindRegistersSave.S:125
+
+#elif defined(__mips__) && _MIPS_SIM == _ABI64
+

Needs checking for soft-float.


https://reviews.llvm.org/D38110



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

I have tested this on one of my machines after removing the checks for soft 
float (my debian install doesn't have the necessary headers for soft-float). 
With the patch you've pointed out and my inline comments addressed (bar the HI 
/ LO register comments), it passes the supplied test suite.

Can you change the checks for ABI from '_MIPS_SIM == $ABI' to 'defined($ABI)'? 
I mistakenly assumed they were always defined and that _MIPS_SIM was defined to 
be one of them but they are only defined when that particular ABI is being used.




Comment at: include/libunwind.h:584
+  UNW_MIPS_R31 = 31,
+};
+

Requires HI / LO registers.



Comment at: src/Registers.hpp:2014
+uint32_t __pc;
+  };
+

This appears to be missing the HI / LO registers.



Comment at: src/Registers.hpp:2039
+return true;
+  // FIXME: Hard float
+  return false;

FIXME: Hard float, DSP accumulator registers, MSA registers



Comment at: src/UnwindRegistersSave.S:120-122
+  jr   $31
+  # fp (in delay slot)
+  sw$30, (4 * 30)($4)

Move the last store out of the delay slot and put 'or $2, $zero, $zero' in the 
delay slot to return UNW_ESUCCESS.



Comment at: src/UnwindRegistersSave.S:155-157
+  jr   $31
+  # fp (in delay slot)
+  sd$30, (8 * 30)($4)

Move the last store out of the delay slot and put 'or $2, $zero, $zero' in the 
delay slot to return UNW_ESUCCESS.


https://reviews.llvm.org/D38110



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis requested changes to this revision.
sdardis added a comment.
This revision now requires changes to proceed.

Marking this as changes required to clear up my review queue - no further 
comments.


https://reviews.llvm.org/D38110



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a reviewer: compnerd.
sdardis added a subscriber: compnerd.
sdardis added a comment.

Two last inlined comments and I think that's everything. @compnerd Have I 
missed anything?




Comment at: src/UnwindRegistersSave.S:100
+#
+DEFINE_LIBUNWIND_FUNCTION(unw_getcontext)
+  .set push

After looking at another implementation of libunwind and the other platforms 
that are supported, it seems to me that we need to save all the registers.



Comment at: src/UnwindRegistersSave.S:142
+#
+DEFINE_LIBUNWIND_FUNCTION(unw_getcontext)
+  .set push

Similarly here as well.


https://reviews.llvm.org/D38110



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: include/__libunwind_config.h:59-68
+# elif defined(__mips__) && defined(_ABIN32) && defined(__mips_soft_float)
+#  define _LIBUNWIND_TARGET_MIPS_N64 1
+#  define _LIBUNWIND_CONTEXT_SIZE 35
+#  define _LIBUNWIND_CURSOR_SIZE 46
+#  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 66
 # elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
 #  define _LIBUNWIND_TARGET_MIPS_N64 1

Change the define of _LIBUNWIND_TARGET_MIPS_N64 to 
_LIBUNWIND_TARGET_MIPS_NEWABI, then these two elif branches can have the 
condition (defined(_ABIN32) && defined(_ABI64) and refactored into one elif 
branch.



Comment at: include/__libunwind_config.h:62
+#  define _LIBUNWIND_CONTEXT_SIZE 35
+#  define _LIBUNWIND_CURSOR_SIZE 46
+#  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 66

Shouldn't this 46 be 47?



Comment at: src/AddressSpace.hpp:201
+inline uint64_t LocalAddressSpace::getRegister(pint_t addr) {
+#if defined(__LP64__) || defined(__mips_n32)
+  return get64(addr);

   defined(__mips_n32) -> (defined(__mips__) && defined(_ABIN32))



Comment at: src/UnwindRegistersRestore.S:548
 
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) && 
defined(__mips_soft_float)
 

This line is overly long, break with '\' after the second &&.



Comment at: src/UnwindRegistersSave.S:146
 
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) && 
defined(__mips_soft_float)
 

This line looks overly long, break before the second &&.



Comment at: src/libunwind.cpp:63-66
+#elif defined(__mips__) && defined(_ABIN32) && defined(__mips_soft_float)
+# define REGISTER_KIND Registers_mips_n64
 #elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
 # define REGISTER_KIND Registers_mips_n64

Fold these two branches together and have the condition (defined(_ABIN32) || 
defined(_ABI64), then follow-up by renaming Registers_mips_n64 to 
Register_mips_NEWABI. Add a comment to the definition of that class stating 
that it covers both n32 and n64.


https://reviews.llvm.org/D39074



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51356: [docs][mips] Clang 7.0 Release notes

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: ReleaseNotes.rst:121
+- On MIPS FreeBSD default CPUs have been changed to ``mips2``
+  for 32-bit targets and ``mips3`` for 32-bit targets.
+

``mips3`` for 64 bit targets.

D48499.


Repository:
  rC Clang

https://reviews.llvm.org/D51356



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Ok, I've found my issue. inttypes.h in libcxx include_next's , 
which on my debian linux systems has the include chain , , 
.  "helpfully" provides #defines of the various _ABIXXX 
macros, which normally the compiler defines depending on the ABI. The include 
chain for other parts of libunwind differ which means 
src/UnwindLevel1-gcc-ext.c had a different definition of the stuct 
unw_context_t, which was the version defined for O32. As GCC and clang laid out 
the stack differently for each ABI differently the bug was masked. However for 
N32 at https://reviews.llvm.org/owners/package/3/, the layout was such that the 
unwind context in the locals area was right below the saved registers for 
_Unwind_Backtrace which trashed older saved values, then triggered a SIGSEGV on 
return due to reloading the saved contents of the $lo  into $ra in my case.

The fix is fairly simple. Change the ABI #ifdef defined() checks to be:

  #ifdef _defined(_ABIO32) && _MIPS_SIM == _ABIO32

for O32 or similar for a single ABI. For N32 and N64 together, it's sufficient 
to test for the presence of __mips64.




Comment at: include/__libunwind_config.h:74
 # elif defined(__mips__)
 #  if defined(_ABIO32) && defined(__mips_soft_float)
 #define _LIBUNWIND_TARGET_MIPS_O32 1

See my comment about the N32 abi check.



Comment at: include/__libunwind_config.h:78
 #define _LIBUNWIND_CURSOR_SIZE 24
+#  elif defined(_ABIN32) && defined(__mips_soft_float)
+#define _LIBUNWIND_TARGET_MIPS_NEWABI 1

This needs to be:

 
```
   #elif defined(_ABIN32) && _MIPS_SIM == _ABIN32
```



Comment at: include/__libunwind_config.h:82
+#define _LIBUNWIND_CURSOR_SIZE 42
 #  elif defined(_ABI64) && defined(__mips_soft_float)
 #define _LIBUNWIND_TARGET_MIPS_NEWABI 1

Same here.



Comment at: src/AddressSpace.hpp:233
+inline uint64_t LocalAddressSpace::getRegister(pint_t addr) {
+#if __SIZEOF_POINTER__ == 8 || (defined(__mips__) && defined(_ABIN32))
+  return get64(addr);

Change that mips check to defined(__mips64) which covers the target being mips 
and it being n64 or n32.



Comment at: src/UnwindRegistersRestore.S:688
 
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) &&
\
+defined(__mips_soft_float)

Rather checking that the _ABI64 or _ABIN32 are defined, check that __mips64 is 
defined.



Comment at: src/UnwindRegistersSave.S:175
 
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) &&
\
+defined(__mips_soft_float)

See my comment on register restore file.



Comment at: src/libunwind.cpp:66
 # define REGISTER_KIND Registers_mips_o32
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABIN32) || defined(_ABI64)) &&
\
+defined(__mips_soft_float)

Check that __mips64 is defined rather than the specific _ABI macros.


https://reviews.llvm.org/D39074



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41968: [libunwind][MIPS] Support MIPS floating-point registers for hard-float ABIs.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a reviewer: compnerd.
sdardis added a comment.

I am not sure what the answer is. There's a slightly different issue in that 
returning the contents of a floating point register on MIPS and expecting it to 
be a double is not necessarily correct. For a 64bit FPU, the result of 
lwc1/mtc1 leaves the upper 32bits of an floating point register as 
**UNPREDICTABLE** by the architecture definition. It's entirely possible for 
the application being unwound to be operating on single precision values, but 
the contents of the register in it's entirety are a signalling nan.

I think the least worst option for registers is to always present the data as 
double values and to only permit access to the sets of double registers for 
O32, and leave the API user to determine how to proceed.

Unfortunately this complicates unwinding in the case of 
https://reviews.llvm.org/source/compiler-rt/ in strict fp32 mode as that 
doesn't have double precision at all. But there's more work to be done for 
https://reviews.llvm.org/source/compiler-rt/ support anyway, so it can be 
deferred. Some nits inlined. Also, watch out for ABI guards such as 
(defined(_ABIN32) || defined(_ABI64), these can be replaced with __mips64.




Comment at: src/Registers.hpp:2659
+  uint32_t _padding;
+  double _floats[32];
+#endif

bsdjhb wrote:
> I chose to always use double here to avoid having different context sizes for 
> the 32-bit vs 64-bit FPR cases.
Add a comment highlighting that design choice.



Comment at: src/UnwindRegistersRestore.S:647
+#if __mips_fpr == 32
+  l.d   $f0, (4 * 36 + 8 * 0)($4)
+  l.d   $f2, (4 * 36 + 8 * 2)($4)

Use ldc1 instead of l.d . l.d is an alias of ldc1.



Comment at: src/UnwindRegistersRestore.S:755
+#ifdef __mips_hard_float
+  l.d   $f0, (8 * 35)($4)
+  l.d   $f1, (8 * 36)($4)

ldc1 here too.



Comment at: src/UnwindRegistersSave.S:119
 
-#elif defined(__mips__) && defined(_ABIO32) && defined(__mips_soft_float)
+#elif defined(__mips__) && defined(_ABIO32)
 

&& _MIPS_SIM == _ABIO32



Comment at: src/UnwindRegistersSave.S:172
+#if __mips_fpr == 32
+  s.d   $f0, (4 * 36 + 8 * 0)($4)
+  s.d   $f2, (4 * 36 + 8 * 2)($4)

Use sdc1 rather than s.d.



Comment at: src/UnwindRegistersSave.S:228
 
-#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) &&
\
-defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32))
 

defined(__mips64) rather than the checking if the ABI names are defined.



Comment at: src/UnwindRegistersSave.S:280
+#ifdef __mips_hard_float
+  s.d   $f0, (8 * 35)($4)
+  s.d   $f1, (8 * 36)($4)

sdc1 rather s.d



Comment at: src/libunwind.cpp:64
 # define REGISTER_KIND Registers_or1k
-#elif defined(__mips__) && defined(_ABIO32) && defined(__mips_soft_float)
+#elif defined(__mips__) && defined(_ABIO32)
 # define REGISTER_KIND Registers_mips_o32

Check for that _MIPS_SIM == _ABIO32 as well.



Comment at: src/libunwind.cpp:66
 # define REGISTER_KIND Registers_mips_o32
-#elif defined(__mips__) && (defined(_ABIN32) || defined(_ABI64)) &&
\
-defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABIN32) || defined(_ABI64))
 # define REGISTER_KIND Registers_mips_newabi

Check for __mips64 rather than defined(_ABIXXX).


https://reviews.llvm.org/D41968



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

The only thing this needs now is to get correct testing support. Could you add 
support for passing down to the configuration script an additional set of 
cflags like compiler-rt and libomp do (as a separate patch)? If you look at 
libomp, you'll see LIBOMP_TEST_CFLAGS defined in the cmake build system and 
threaded through various places so the built tests can have the correct 
options. Without supporting that we get a mixed set of objects, which isn't 
correct.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D39074



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Yes. Something like LIBUNWIND_TEST_CFLAGS mapping to config.test_cflags, which 
libunwind/test/libunwind/test/config.py then appends when building the tests.

Thanks.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D39074



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43487: [mips] Spectre variant two mitigation for MIPSR2

[Simon Dardis via Phabricator via cfe-commits]

sdardis created this revision.
sdardis added a reviewer: atanasyan.
Herald added a subscriber: arichardson.

This patch provides migitation for CVE-2017-5715, Spectre variant two,
which affects the https://reviews.llvm.org/P5600 and 
https://reviews.llvm.org/P6600. It provides the option
-mindirect-jump=hazard, which instructs the LLVM backend to replace
indirect branches with their hazard barrier variants.

This option is accepted when targetting a MIPS revision two or later.

The migitation strategy suggested by MIPS for these processors is to use
two hazard barrier instructions. 'jalr.hb' and 'jr.hb' are hazard
barrier variants of the 'jalr' and 'jr' instructions respectively.

These instructions impede the execution of instruction stream until
architecturally defined hazards (changes to the instruction stream,
privileged registers which may affect execution) are cleared. These
instructions in MIPS' designs are not speculated past.

These instructions are used with the attribute +use-indirect-jump-hazard
when branching indirectly and for indirect function calls.

These instructions are defined by the MIPS32R2 ISA, so this mitigation
method is not compatible with processors which implement an earlier
revision of the MIPS ISA.

Implementation note: I've opted to provide this as an
-mindirect-jump={hazard,...} style option in case alternative
mitigation methods are required for other implementations of the MIPS ISA
in future, e.g. retpoline style solutions.


Repository:
  rC Clang

https://reviews.llvm.org/D43487

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  lib/Basic/Targets/Mips.h
  lib/Driver/ToolChains/Arch/Mips.cpp
  lib/Driver/ToolChains/Arch/Mips.h
  test/Driver/mips-features.c
  test/Driver/mips-indirect-branch.c

Index: test/Driver/mips-indirect-branch.c
===
--- /dev/null
+++ test/Driver/mips-indirect-branch.c
@@ -0,0 +1,23 @@
+// REQUIRES: mips-registered-target
+// -mindirect-jump=hazard -mips32
+// RUN: %clang -target mips-unknown-linux-gnu -mips32 -### -c %s \
+// RUN:-mindirect-jump=hazard 2>&1 | FileCheck %s --check-prefix=MIPS32
+// MIPS32: error: '-mindirect-jump=hazard' is unsupported with the 'mips32' architecture
+
+// -mindirect-jump=hazard -mmicromips
+// RUN: %clang -target mips-unknown-linux-gnu -mmicromips -### -c %s \
+// RUN:-mindirect-jump=hazard 2>&1 | FileCheck %s --check-prefix=MICROMIPS
+// MICROMIPS: error: '-mindirect-jump=hazard' is unsupported with the 'micromips' architecture
+
+// -mindirect-jump=hazard -mips16
+// RUN: %clang -target mips-unknown-linux-gnu -mips16 -### -c %s \
+// RUN:-mindirect-jump=hazard 2>&1 | FileCheck %s --check-prefix=MIPS16
+// MIPS16: error: '-mindirect-jump=hazard' is unsupported with the 'mips16' architecture
+
+// RUN: %clang -target mips-unknown-linux-gnu  -### -c %s \
+// RUN:-mindirect-jump=retopline 2>&1 | FileCheck %s --check-prefix=RETOPLINE
+// RETOPLINE: error: unknown '-mindirect-jump=' option 'retopline'
+
+// RUN: %clang -target mips-unknown-linux-gnu  -### -mips32 -c %s \
+// RUN:-mindirect-jump=retopline 2>&1 | FileCheck %s --check-prefix=MIXED
+// MIXED: error: unknown '-mindirect-jump=' option 'retopline'
Index: test/Driver/mips-features.c
===
--- test/Driver/mips-features.c
+++ test/Driver/mips-features.c
@@ -402,3 +402,9 @@
 // RUN: %clang -target -mips-mti-linux-gnu -### -c %s -mno-branch-likely 2>&1 \
 // RUN:   | FileCheck --check-prefix=NO-BRANCH-LIKELY %s
 // NO-BRANCH-LIKELY: argument unused during compilation: '-mno-branch-likely'
+
+// -mindirect-jump=hazard
+// RUN: %clang -target mips-unknown-linux-gnu -### -c %s \
+// RUN:-mindirect-jump=hazard 2>&1 \
+// RUN:   | FileCheck --check-prefix=INDIRECT-BH %s
+// INDIRECT-BH: "-target-feature" "+use-indirect-jump-hazard"
Index: lib/Driver/ToolChains/Arch/Mips.h
===
--- lib/Driver/ToolChains/Arch/Mips.h
+++ lib/Driver/ToolChains/Arch/Mips.h
@@ -53,6 +53,7 @@
 bool shouldUseFPXX(const llvm::opt::ArgList &Args, const llvm::Triple &Triple,
StringRef CPUName, StringRef ABIName,
mips::FloatABI FloatABI);
+bool supportsIndirectJumpHazardBarrier(StringRef &CPU);
 
 } // end namespace mips
 } // end namespace target
Index: lib/Driver/ToolChains/Arch/Mips.cpp
===
--- lib/Driver/ToolChains/Arch/Mips.cpp
+++ lib/Driver/ToolChains/Arch/Mips.cpp
@@ -343,6 +343,28 @@
   AddTargetFeature(Args, Features, options::OPT_mno_madd4, options::OPT_mmadd4,
"nomadd4");
   AddTargetFeature(Args, Features, options::OPT_mmt, options::OPT_mno_mt, "mt");
+
+  if (Arg *A = Args.getLastArg(options::OPT_mindirect_jump_EQ)) {
+StringRef Val = StringRef(A->getValue());
+if (Val == "hazard") {

[PATCH] D43509: [libcxx][cmake] Remove libatomic temporarily from CMAKE_REQUIRED_LIBRARIES when configuring

[Simon Dardis via Phabricator via cfe-commits]

sdardis created this revision.
sdardis added a reviewer: EricWF.
Herald added subscribers: christof, mgorny.

When libcxx is built in tree for a host which requires libatomic, LLVM's
configuration steps will determine it is required and add it to
CMAKE_REQUIRED_LIBRARIES. When libcxx is later configured, it tests if it
has C++ atomics without libatomic. The test erroneously passes as libatomic
is already part of the set of required libraries.

In turn, a number of the atomic tests will fail as they require libatomic
but the test suite is configured not to use libatomic.

Address this by always dropping libatomic from the set of required libraries
before determining if LIBCXX_HAVE_CXX_ATOMICS_WITHOUT_LIB is true,
then restoring the set of required libraries.


Repository:
  rCXX libc++

https://reviews.llvm.org/D43509

Files:
  cmake/Modules/CheckLibcxxAtomic.cmake


Index: cmake/Modules/CheckLibcxxAtomic.cmake
===
--- cmake/Modules/CheckLibcxxAtomic.cmake
+++ cmake/Modules/CheckLibcxxAtomic.cmake
@@ -31,7 +31,14 @@
   set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
 endfunction(check_cxx_atomics)
 
+# Perform the check for 64bit atomics without libatomic. It may have been added
+# to the required libraries earlier in the configuration, which would cause
+# this check to incorrectly pass.
+set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
+list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES "atomic")
 check_cxx_atomics(LIBCXX_HAVE_CXX_ATOMICS_WITHOUT_LIB)
+set(CMAKE_REQUIRED_LIBRARIES ${OLD_CMAKE_REQUIRED_LIBRARIES})
+
 check_library_exists(atomic __atomic_fetch_add_8 "" LIBCXX_HAS_ATOMIC_LIB)
 # If not, check if the library exists, and atomics work with it.
 if(NOT LIBCXX_HAVE_CXX_ATOMICS_WITHOUT_LIB)


Index: cmake/Modules/CheckLibcxxAtomic.cmake
===
--- cmake/Modules/CheckLibcxxAtomic.cmake
+++ cmake/Modules/CheckLibcxxAtomic.cmake
@@ -31,7 +31,14 @@
   set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
 endfunction(check_cxx_atomics)
 
+# Perform the check for 64bit atomics without libatomic. It may have been added
+# to the required libraries earlier in the configuration, which would cause
+# this check to incorrectly pass.
+set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
+list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES "atomic")
 check_cxx_atomics(LIBCXX_HAVE_CXX_ATOMICS_WITHOUT_LIB)
+set(CMAKE_REQUIRED_LIBRARIES ${OLD_CMAKE_REQUIRED_LIBRARIES})
+
 check_library_exists(atomic __atomic_fetch_add_8 "" LIBCXX_HAS_ATOMIC_LIB)
 # If not, check if the library exists, and atomics work with it.
 if(NOT LIBCXX_HAVE_CXX_ATOMICS_WITHOUT_LIB)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43487: [mips] Spectre variant two mitigation for MIPSR2

[Simon Dardis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL325651: [mips] Spectre variant two mitigation for MIPSR2 
(authored by sdardis, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D43487

Files:
  cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Basic/Targets/Mips.h
  cfe/trunk/lib/Driver/ToolChains/Arch/Mips.cpp
  cfe/trunk/lib/Driver/ToolChains/Arch/Mips.h
  cfe/trunk/test/Driver/mips-features.c
  cfe/trunk/test/Driver/mips-indirect-branch.c

Index: cfe/trunk/test/Driver/mips-features.c
===
--- cfe/trunk/test/Driver/mips-features.c
+++ cfe/trunk/test/Driver/mips-features.c
@@ -402,3 +402,9 @@
 // RUN: %clang -target -mips-mti-linux-gnu -### -c %s -mno-branch-likely 2>&1 \
 // RUN:   | FileCheck --check-prefix=NO-BRANCH-LIKELY %s
 // NO-BRANCH-LIKELY: argument unused during compilation: '-mno-branch-likely'
+
+// -mindirect-jump=hazard
+// RUN: %clang -target mips-unknown-linux-gnu -### -c %s \
+// RUN:-mindirect-jump=hazard 2>&1 \
+// RUN:   | FileCheck --check-prefix=INDIRECT-BH %s
+// INDIRECT-BH: "-target-feature" "+use-indirect-jump-hazard"
Index: cfe/trunk/test/Driver/mips-indirect-branch.c
===
--- cfe/trunk/test/Driver/mips-indirect-branch.c
+++ cfe/trunk/test/Driver/mips-indirect-branch.c
@@ -0,0 +1,23 @@
+// REQUIRES: mips-registered-target
+// -mindirect-jump=hazard -mips32
+// RUN: %clang -target mips-unknown-linux-gnu -mips32 -### -c %s \
+// RUN:-mindirect-jump=hazard 2>&1 | FileCheck %s --check-prefix=MIPS32
+// MIPS32: error: '-mindirect-jump=hazard' is unsupported with the 'mips32' architecture
+
+// -mindirect-jump=hazard -mmicromips
+// RUN: %clang -target mips-unknown-linux-gnu -mmicromips -### -c %s \
+// RUN:-mindirect-jump=hazard 2>&1 | FileCheck %s --check-prefix=MICROMIPS
+// MICROMIPS: error: '-mindirect-jump=hazard' is unsupported with the 'micromips' architecture
+
+// -mindirect-jump=hazard -mips16
+// RUN: %clang -target mips-unknown-linux-gnu -mips16 -### -c %s \
+// RUN:-mindirect-jump=hazard 2>&1 | FileCheck %s --check-prefix=MIPS16
+// MIPS16: error: '-mindirect-jump=hazard' is unsupported with the 'mips16' architecture
+
+// RUN: %clang -target mips-unknown-linux-gnu  -### -c %s \
+// RUN:-mindirect-jump=retopline 2>&1 | FileCheck %s --check-prefix=RETOPLINE
+// RETOPLINE: error: unknown '-mindirect-jump=' option 'retopline'
+
+// RUN: %clang -target mips-unknown-linux-gnu  -### -mips32 -c %s \
+// RUN:-mindirect-jump=retopline 2>&1 | FileCheck %s --check-prefix=MIXED
+// MIXED: error: unknown '-mindirect-jump=' option 'retopline'
Index: cfe/trunk/lib/Driver/ToolChains/Arch/Mips.h
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/Mips.h
+++ cfe/trunk/lib/Driver/ToolChains/Arch/Mips.h
@@ -53,6 +53,7 @@
 bool shouldUseFPXX(const llvm::opt::ArgList &Args, const llvm::Triple &Triple,
StringRef CPUName, StringRef ABIName,
mips::FloatABI FloatABI);
+bool supportsIndirectJumpHazardBarrier(StringRef &CPU);
 
 } // end namespace mips
 } // end namespace target
Index: cfe/trunk/lib/Driver/ToolChains/Arch/Mips.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/Mips.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/Mips.cpp
@@ -343,6 +343,28 @@
   AddTargetFeature(Args, Features, options::OPT_mno_madd4, options::OPT_mmadd4,
"nomadd4");
   AddTargetFeature(Args, Features, options::OPT_mmt, options::OPT_mno_mt, "mt");
+
+  if (Arg *A = Args.getLastArg(options::OPT_mindirect_jump_EQ)) {
+StringRef Val = StringRef(A->getValue());
+if (Val == "hazard") {
+  Arg *B =
+  Args.getLastArg(options::OPT_mmicromips, options::OPT_mno_micromips);
+  Arg *C = Args.getLastArg(options::OPT_mips16, options::OPT_mno_mips16);
+
+  if (B && B->getOption().matches(options::OPT_mmicromips))
+D.Diag(diag::err_drv_unsupported_indirect_jump_opt)
+<< "hazard" << "micromips";
+  else if (C && C->getOption().matches(options::OPT_mips16))
+D.Diag(diag::err_drv_unsupported_indirect_jump_opt)
+<< "hazard" << "mips16";
+  else if (mips::supportsIndirectJumpHazardBarrier(CPUName))
+Features.push_back("+use-indirect-jump-hazard");
+  else
+D.Diag(diag::err_drv_unsupported_indirect_jump_opt)
+<< "hazard" << CPUName;
+} else
+  D.Diag(diag::err_drv_unknown_indirect_jump_opt) << Val;
+  }
 }
 
 mips::IEEE754Standard mips::getIEEE754Standard(StringRef &CPU) {
@@ -447,3 +469,20 @@
 
   return UseFPXX;
 }
+
+bool mips::supportsIndirectJumpHazardBarrier(StringRef &CPU) {
+  // Supporting the hazard barri

[PATCH] D43585: [libunwind] Permit additional compiler flags to be passed to tests.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a subscriber: arichardson.
sdardis added a comment.

This works as expected with your corresponding libcxx patch. However 
@arichardson 's https://reviews.llvm.org/D42139 does similar things for libcxx. 
We should wait for that to land, and ensure that this patch is rebased against 
that.




Comment at: CMakeLists.txt:142
 set(LIBUNWIND_SYSROOT "" CACHE PATH "Sysroot for cross compiling.")
+set(LIBUNWIND_TEST_CFLAGS "" CACHE PATH "Additional compiler flags for test 
programs.")
 

PATH should be STRING.



Comment at: test/lit.site.cfg.in:25
 config.cxx_ext_threads  = "@LIBUNWIND_BUILD_EXTERNAL_THREAD_LIBRARY@"
+config.test_cflags  = "@LIBUNWIND_TEST_CFLAGS@"
 

This should be test_compiler_cflags.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D43585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43585: [libunwind] Permit additional compiler flags to be passed to tests.

[Simon Dardis via Phabricator via cfe-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D43585#1017957, @bsdjhb wrote:

> My only question is if this should be named LIBUNWIND_TEST_COMPILER_FLAGS to 
> match the name used in libc++?


I would say yes for the sake of consistency with the option it's controls and 
libcxx. Additionally, we also require config.test_linker_flags to be set as 
well, so we need LIBUNWIND_LINKER_FLAGS.

LGTM with those two additional changes. I've tested it with your N32 patch and 
it works.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D43585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: christof.

LGTM after  https://reviews.llvm.org/D43585 lands.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D39074



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33401: [mips] Add runtime options to enable/disable generation of madd.fmt, msub.fmt

[Simon Dardis via Phabricator via cfe-commits]

sdardis requested changes to this revision.
sdardis added a comment.
This revision now requires changes to proceed.

This also requires that __mips_no_madd4 is defined when the -mnomadd4 option is 
in use.


https://reviews.llvm.org/D33401



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33401: [mips] Add runtime options to enable/disable generation of madd.fmt, msub.fmt

[Simon Dardis via Phabricator via cfe-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

The new define also requires a test in test/Preprocessor/init.c - test that by 
default the new define isn't present, and in some case, when supplied with the 
-mno-madd4 that it is present.

LGTM with that change.


https://reviews.llvm.org/D33401



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34514: [mips] Enable IAS by default for Android 64-bit MIPS target (N64)

[Simon Dardis via Phabricator via cfe-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

LGTM with a comment clarity tweak.




Comment at: lib/Driver/ToolChains/Gnu.cpp:2322-2323
   case llvm::Triple::mips64el:
-// Enabled for Debian mips64/mips64el only. Other targets are unable to
-// distinguish N32 from N64.
-if (getTriple().getEnvironment() == llvm::Triple::GNUABI64)
+// Enabled for Debian mips64/mips64el and Android (uses N64 ABI).
+// Other targets are unable to distinguish N32 from N64.
+if (getTriple().getEnvironment() == llvm::Triple::GNUABI64 ||

Enabled for Debian and Android mips64/mipsel, as they can precisely
identify the ABI in use (Debian) or only use N64 for MIPS64 (Android).
Other targets are unable to distinguish N32 from N64.


Repository:
  rL LLVM

https://reviews.llvm.org/D34514



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43509: [libcxx][cmake] Remove libatomic temporarily from CMAKE_REQUIRED_LIBRARIES when configuring

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Ping.


Repository:
  rCXX libc++

https://reviews.llvm.org/D43509



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43509: [libcxx][cmake] Remove libatomic temporarily from CMAKE_REQUIRED_LIBRARIES when configuring

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Ping.


Repository:
  rCXX libc++

https://reviews.llvm.org/D43509



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44248: [clangd][cmake] Provide libatomic when there is no native support for 64bit atomics

[Simon Dardis via Phabricator via cfe-commits]

sdardis created this revision.
Herald added subscribers: ioeric, jkorous-apple, ilya-biryukov, arichardson, 
mgorny, klimek.

This addresses a persistent failure on clang-cmake-mips buildbot.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44248

Files:
  clangd/CMakeLists.txt


Index: clangd/CMakeLists.txt
===
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -2,6 +2,11 @@
   Support
   )
 
+set(CLANGD_ATOMIC_LIB "")
+if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
+  list(APPEND CLANGD_ATOMIC_LIB atomic)
+endif()
+
 add_clang_library(clangDaemon
   ClangdLSPServer.cpp
   ClangdServer.cpp
@@ -48,6 +53,7 @@
   clangToolingCore
   clangToolingRefactor
   ${LLVM_PTHREAD_LIB}
+  ${CLANGD_ATOMIC_LIB}
   )
 
 if( LLVM_LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )


Index: clangd/CMakeLists.txt
===
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -2,6 +2,11 @@
   Support
   )
 
+set(CLANGD_ATOMIC_LIB "")
+if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
+  list(APPEND CLANGD_ATOMIC_LIB atomic)
+endif()
+
 add_clang_library(clangDaemon
   ClangdLSPServer.cpp
   ClangdServer.cpp
@@ -48,6 +53,7 @@
   clangToolingCore
   clangToolingRefactor
   ${LLVM_PTHREAD_LIB}
+  ${CLANGD_ATOMIC_LIB}
   )
 
 if( LLVM_LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44381: [mips] Change the way how Clang chooses relocation mode

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

This patch looks mostly ok, but I think there are some small issues with it.

Can you separate the new warnings/error relating to -fno-pic / -mabicalls into 
another patch from the change which alters the behaviour of __PIC__ / __pic__ ?

The title of the patch is misleading, as you're adding warnings related to 
-fno-pic / -mabicalls and changing how __pic__ is defined.


Repository:
  rC Clang

https://reviews.llvm.org/D44381



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44248: [clangd][cmake] Provide libatomic when there is no native support for 64bit atomics

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Ping?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44248



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44248: [clangd][cmake] Provide libatomic when there is no native support for 64bit atomics

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Thanks, I wasn't sure who to add as a reviewer.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44248



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47829: [Driver] Accept the -fno-shrink-wrap option for GCC compatibility

[Simon Dardis via Phabricator via cfe-commits]

sdardis created this revision.
sdardis added a reviewer: rsmith.
Herald added a reviewer: javed.absar.
Herald added subscribers: atanasyan, kristof.beyls, arichardson.

As reported in GCC bug #86069, LLVM currently provokes a bug in GCC where
objects compiled for MIPS with PIC and optimizations where shrink wrapped
functions can attempt to access the global pointer's spill slot before the 
global
pointer is spilled. This bug is present in GCC since at least 4.9.2, and 
affects the
AArch64 backend when LLVM is built for a MIPS host.

However, setting CMAKE_C_FLAGS and CMAKE_CXX_FLAGS affects the compiler-rt
tests which rely on the just built clang rather than the host or cross GCC.
Since Clang doesn't support this flag, so the tests spuriously fail.

Unfortunately, not all targets support shrink wrapping in LLVM at this time,
so providing -fshrink-wrap would expose users to any number of wrong-codegen
issues.

For targets that do support shrink wrapping, this option performs as expected.


Repository:
  rC Clang

https://reviews.llvm.org/D47829

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/shrink-wrap.c


Index: test/Driver/shrink-wrap.c
===
--- /dev/null
+++ test/Driver/shrink-wrap.c
@@ -0,0 +1,12 @@
+// RUN: %clang -### %s -fno-shrink-wrap 2> %t
+// RUN: FileCheck --check-prefix=CHECK-NO < %t %s
+
+// The below case should be handled when shrink wrapping is available for all
+// targets.
+
+// FIXME: %clang -### %s -fshrink-wrap 2> %t
+// FIXME: FileCheck --check-prefix=CHECK-YES < %t %s
+
+// CHECK-NO: "-mllvm" "-enable-shrink-wrap=false"
+
+// CHECK-YES: "-mllvm" "-enable-shrink-wrap=true"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4047,6 +4047,11 @@
 
   Args.AddLastArg(CmdArgs, options::OPT_ftrap_function_EQ);
 
+  if (Args.hasArg(options::OPT_fno_shrink_wrap)) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-enable-shrink-wrap=false");
+  }
+
   // -fno-strict-overflow implies -fwrapv if it isn't disabled, but
   // -fstrict-overflow won't turn off an explicitly enabled -fwrapv.
   if (Arg *A = Args.getLastArg(options::OPT_fwrapv, options::OPT_fno_wrapv)) {
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1388,6 +1388,8 @@
   HelpText<"Do not include column number on diagnostics">;
 def fno_show_source_location : Flag<["-"], "fno-show-source-location">, 
Group,
   Flags<[CC1Option]>, HelpText<"Do not include source location information 
with diagnostics">;
+def fno_shrink_wrap : Flag<["-"], "fno-shrink-wrap">, Group,
+  Flags<[CC1Option]>, HelpText<"Disable shrink wrapping">;
 def fdiagnostics_absolute_paths : Flag<["-"], "fdiagnostics-absolute-paths">, 
Group,
   Flags<[CC1Option, CoreOption]>, HelpText<"Print absolute paths in 
diagnostics">;
 def fno_spell_checking : Flag<["-"], "fno-spell-checking">, Group,


Index: test/Driver/shrink-wrap.c
===
--- /dev/null
+++ test/Driver/shrink-wrap.c
@@ -0,0 +1,12 @@
+// RUN: %clang -### %s -fno-shrink-wrap 2> %t
+// RUN: FileCheck --check-prefix=CHECK-NO < %t %s
+
+// The below case should be handled when shrink wrapping is available for all
+// targets.
+
+// FIXME: %clang -### %s -fshrink-wrap 2> %t
+// FIXME: FileCheck --check-prefix=CHECK-YES < %t %s
+
+// CHECK-NO: "-mllvm" "-enable-shrink-wrap=false"
+
+// CHECK-YES: "-mllvm" "-enable-shrink-wrap=true"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4047,6 +4047,11 @@
 
   Args.AddLastArg(CmdArgs, options::OPT_ftrap_function_EQ);
 
+  if (Args.hasArg(options::OPT_fno_shrink_wrap)) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-enable-shrink-wrap=false");
+  }
+
   // -fno-strict-overflow implies -fwrapv if it isn't disabled, but
   // -fstrict-overflow won't turn off an explicitly enabled -fwrapv.
   if (Arg *A = Args.getLastArg(options::OPT_fwrapv, options::OPT_fno_wrapv)) {
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1388,6 +1388,8 @@
   HelpText<"Do not include column number on diagnostics">;
 def fno_show_source_location : Flag<["-"], "fno-show-source-location">, Group,
   Flags<[CC1Option]>, HelpText<"Do not include source location information with diagnostics">;
+def fno_shrink_wrap : Flag<["-"], "fno-shrink-wrap">, Group,
+  Flags<[CC1Option]>, HelpText<"Disable shrink wrapping">;
 def fdiagnostics_absolute_paths : Flag<["-"], "fdiagnostics-absolute-paths">, Grou

[PATCH] D47829: [Driver] Accept the -fno-shrink-wrap option for GCC compatibility

[Simon Dardis via Phabricator via cfe-commits]

sdardis updated this revision to Diff 150904.
sdardis added a comment.

Modify implementation to produce a function attribute.


Repository:
  rC Clang

https://reviews.llvm.org/D47829

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CodeGenFunction.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/noshrinkwrapping.c


Index: test/CodeGen/noshrinkwrapping.c
===
--- /dev/null
+++ test/CodeGen/noshrinkwrapping.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -S -fno-shrink-wrapping %s -emit-llvm -o - | FileCheck %s
+
+// CHECK-LABEL: main
+// CHECK: attributes #0 = {{.*}}"no-shrink-wrapping"="true"{{.*}}
+
+int main() {
+  return 0;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -743,6 +743,8 @@
 
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
+  Opts.NoShrinkWrap = Args.hasArg(OPT_fno_shrink_wrap);
+
   Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);
 
   Opts.ProfileSampleAccurate = Args.hasArg(OPT_fprofile_sample_accurate);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4054,6 +4054,9 @@
 
   Args.AddLastArg(CmdArgs, options::OPT_ftrap_function_EQ);
 
+  if (Args.hasArg(options::OPT_fno_shrink_wrap))
+CmdArgs.push_back("-fno-shrink-wrap");
+
   // -fno-strict-overflow implies -fwrapv if it isn't disabled, but
   // -fstrict-overflow won't turn off an explicitly enabled -fwrapv.
   if (Arg *A = Args.getLastArg(options::OPT_fwrapv, options::OPT_fno_wrapv)) {
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -935,6 +935,11 @@
 }
   }
 
+  // Add no-shrink-wrap value.
+  if (CGM.getCodeGenOpts().NoShrinkWrap)
+Fn->addFnAttr("no-shrink-wrap",
+  llvm::toStringRef(CGM.getCodeGenOpts().NoShrinkWrap));
+
   // Add no-jump-tables value.
   Fn->addFnAttr("no-jump-tables",
 llvm::toStringRef(CGM.getCodeGenOpts().NoUseJumpTables));
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -216,6 +216,7 @@
 CODEGENOPT(TimePasses, 1, 0) ///< Set when -ftime-report is enabled.
 CODEGENOPT(UnrollLoops   , 1, 0) ///< Control whether loops are unrolled.
 CODEGENOPT(RerollLoops   , 1, 0) ///< Control whether loops are rerolled.
+CODEGENOPT(NoShrinkWrap  , 1, 0) ///< Set when -fno-shrink-wrap is enabled.
 CODEGENOPT(NoUseJumpTables   , 1, 0) ///< Set when -fno-jump-tables is enabled.
 CODEGENOPT(UnsafeFPMath  , 1, 0) ///< Allow unsafe floating point optzns.
 CODEGENOPT(UnwindTables  , 1, 0) ///< Emit unwind tables.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1393,6 +1393,8 @@
   HelpText<"Do not include column number on diagnostics">;
 def fno_show_source_location : Flag<["-"], "fno-show-source-location">, 
Group,
   Flags<[CC1Option]>, HelpText<"Do not include source location information 
with diagnostics">;
+def fno_shrink_wrap : Flag<["-"], "fno-shrink-wrap">, Group,
+  Flags<[CC1Option]>, HelpText<"Disable shrink wrapping">;
 def fdiagnostics_absolute_paths : Flag<["-"], "fdiagnostics-absolute-paths">, 
Group,
   Flags<[CC1Option, CoreOption]>, HelpText<"Print absolute paths in 
diagnostics">;
 def fno_spell_checking : Flag<["-"], "fno-spell-checking">, Group,


Index: test/CodeGen/noshrinkwrapping.c
===
--- /dev/null
+++ test/CodeGen/noshrinkwrapping.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -S -fno-shrink-wrapping %s -emit-llvm -o - | FileCheck %s
+
+// CHECK-LABEL: main
+// CHECK: attributes #0 = {{.*}}"no-shrink-wrapping"="true"{{.*}}
+
+int main() {
+  return 0;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -743,6 +743,8 @@
 
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
+  Opts.NoShrinkWrap = Args.hasArg(OPT_fno_shrink_wrap);
+
   Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);
 
   Opts.ProfileSampleAccurate = Args.hasArg(OPT_fprofile_sample_accurate);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4054,6 +4054,9 @@

[PATCH] D47829: [Driver] Accept the -fno-shrink-wrap option for GCC compatibility

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

In https://reviews.llvm.org/D47829#1124040, @efriedma wrote:

> Is this something which is actually useful to control?  From your 
> description, you want to add the flag to clang not because you actually want 
> to use it, but just because you can't figure out how to pass the right flags 
> to your clang build.
>
> If it is useful, it should be implemented as a function attribute, not a 
> global flag.


This is useful to control in the sense of being able to opt-in or opt-out of 
the shrink wrapping pass for clang, especially in the cases of where we might 
discover a triggerable bug in a released version of clang.

Currently, the compiler-rt sanitizer tests make use of the CMAKE_C_FLAGS and 
CMAKE_CXX_FLAGS when generating the test objects. Working around this would 
require stripping the incompatible arguments from those variables when building 
the test objects. This seems to me to worse choice, as I'm also posting a patch 
to automatically add -fno-shrink-wrap when building LLVM with GCC for MIPS 
(https://reviews.llvm.org/D48069).


Repository:
  rC Clang

https://reviews.llvm.org/D47829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48169: [mips] Add '-mcrc', '-mno-crc' options to enable/disable CRC ASE

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: include/clang/Driver/Options.td:2214
   HelpText<"Disable SVR4-style position-independent code (Mips only)">;
+def mno_crc : Flag<["-"], "mno-crc">, Group,
+  HelpText<"Disallow use of CRC instructions (Mips only)">;

atanasyan wrote:
> Does it make a sense to define this option as an alias to the `mnocrc` and 
> allow to use both `-mno-crc` and `-mnocrc` for ARM and MIPS?
I thought about commenting on that. The problem there is that you then 
permitting clang to be more lax than binutils which inhibits compatibility 
between the two tools.


Repository:
  rC Clang

https://reviews.llvm.org/D48169



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56878: [mips] Add '-mrelax-pic-calls', '-mno-relax-pic-calls'

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: cfe/trunk/include/clang/Driver/Options.td:2423
+  Group,
+  HelpText<"Try turning PIC calls (j{al}r{c} $25) into direct calls "
+  "(MIPS only)">, Flags<[HelpHidden]>;

I think this help text could be a little better. Instead of implying that the 
compiler could turn PIC calls into direct calls, something like "Produce 
relaxation hints for linkers to try optimizing PIC call sequences into direct 
calls", I believe describes the optimization better. Likewise in the negative 
sense for the -mno-relax-pic-calls.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56878/new/

https://reviews.llvm.org/D56878



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38168: [mips] Accept but ignore -m(no-)branch-likely

[Simon Dardis via Phabricator via cfe-commits]

sdardis created this revision.
Herald added a subscriber: arichardson.

-mbranch-likely and -mno-branch-likely are used in some build systems for
some MIPS targets. Accept these options but ignore them as they are an
(de)optimiztion hint, and that branch likely instructions were deprecated
but not removed from MIPS32 and MIPS64 ISAs.


https://reviews.llvm.org/D38168

Files:
  include/clang/Driver/Options.td
  test/Driver/mips-features.c


Index: test/Driver/mips-features.c
===
--- test/Driver/mips-features.c
+++ test/Driver/mips-features.c
@@ -392,3 +392,13 @@
 // LONG-CALLS-ON: "-target-feature" "+long-calls"
 // LONG-CALLS-OFF: "-target-feature" "-long-calls"
 // LONG-CALLS-DEF-NOT: "long-calls"
+//
+// -mbranch-likely
+// RUN: %clang -target -mips-mti-linux-gnu -### -c %s -mbranch-likely 2>&1 \
+// RUN:   | FileCheck --check-prefix=BRANCH-LIKELY %s
+// BRANCH-LIKELY: argument unused during compilation: '-mbranch-likely'
+//
+// -mno-branch-likely
+// RUN: %clang -target -mips-mti-linux-gnu -### -c %s -mno-branch-likely 2>&1 \
+// RUN:   | FileCheck --check-prefix=NO-BRANCH-LIKELY %s
+// NO-BRANCH-LIKELY: argument unused during compilation: '-mno-branch-likely'
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -2047,6 +2047,10 @@
 def mno_check_zero_division : Flag<["-"], "mno-check-zero-division">,
   Group;
 def mcompact_branches_EQ : Joined<["-"], "mcompact-branches=">, Group;
+def mbranch_likely : Flag<["-"], "mbranch-likely">, Group,
+  IgnoredGCCCompat;
+def mno_branch_likely : Flag<["-"], "mno-branch-likely">, Group,
+  IgnoredGCCCompat;
 def mdsp : Flag<["-"], "mdsp">, Group;
 def mno_dsp : Flag<["-"], "mno-dsp">, Group;
 def mdspr2 : Flag<["-"], "mdspr2">, Group;


Index: test/Driver/mips-features.c
===
--- test/Driver/mips-features.c
+++ test/Driver/mips-features.c
@@ -392,3 +392,13 @@
 // LONG-CALLS-ON: "-target-feature" "+long-calls"
 // LONG-CALLS-OFF: "-target-feature" "-long-calls"
 // LONG-CALLS-DEF-NOT: "long-calls"
+//
+// -mbranch-likely
+// RUN: %clang -target -mips-mti-linux-gnu -### -c %s -mbranch-likely 2>&1 \
+// RUN:   | FileCheck --check-prefix=BRANCH-LIKELY %s
+// BRANCH-LIKELY: argument unused during compilation: '-mbranch-likely'
+//
+// -mno-branch-likely
+// RUN: %clang -target -mips-mti-linux-gnu -### -c %s -mno-branch-likely 2>&1 \
+// RUN:   | FileCheck --check-prefix=NO-BRANCH-LIKELY %s
+// NO-BRANCH-LIKELY: argument unused during compilation: '-mno-branch-likely'
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -2047,6 +2047,10 @@
 def mno_check_zero_division : Flag<["-"], "mno-check-zero-division">,
   Group;
 def mcompact_branches_EQ : Joined<["-"], "mcompact-branches=">, Group;
+def mbranch_likely : Flag<["-"], "mbranch-likely">, Group,
+  IgnoredGCCCompat;
+def mno_branch_likely : Flag<["-"], "mno-branch-likely">, Group,
+  IgnoredGCCCompat;
 def mdsp : Flag<["-"], "mdsp">, Group;
 def mno_dsp : Flag<["-"], "mno-dsp">, Group;
 def mdspr2 : Flag<["-"], "mdspr2">, Group;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55878: [Driver] Use --hash-style=gnu instead of both on FreeBSD

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

> I can't find rationale behind the MIPS discrepancy in the original commit. I 
> can add the if branch back if you tell me why...

From my recollections, the gnu-hash style isn't supported in ld.bfd for MIPS. A 
patch was posted to the binutils list 
(https://sourceware.org/ml/binutils/2015-10/msg00057.html) to support this, but 
there was an issue with FSF copyright assignment. As the patch is non-trivial, 
it would have to be independently re-developed for submission to binutils and 
later lld.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55878/new/

https://reviews.llvm.org/D55878



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44381: [mips] Prevent PIC to be set to level 2

[Simon Dardis via Phabricator via cfe-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D44381



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44684: [mips] Improve handling of -fno-[pic/PIC] option

[Simon Dardis via Phabricator via cfe-commits]

sdardis requested changes to this revision.
sdardis added a comment.
This revision now requires changes to proceed.

A quick comment on the error message, inlined. It's about the quality of the 
diagnostics.




Comment at: include/clang/Basic/DiagnosticDriverKinds.td:336-337
+def warn_drv_unsupported_pic : Warning<
+  "ignoring '-fno-pic' option as it cannot be used with "
+  "-mabicalls and the N64 ABI">,
   InGroup;

Can you fine tune this error message to say:

"ignoring '-fno-pic' option as it cannot be used with explicit use of 
-mabicalls and the N64 ABI" when -mabicalls is used on the commandline and:

"ignoring '-fno-pic' option as it cannot be used with implicit use of 
-mabicalls and the N64 ABI" when -mno-abicalls and -mabicalls are not present.

You should also report the precise pic/PIC/pie/PIE option used in the error 
message. You should be able to get it from the argument directly: 
http://llvm.org/doxygen/classllvm_1_1opt_1_1Arg.html


Repository:
  rC Clang

https://reviews.llvm.org/D44684



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44684: [mips] Improve handling of -fno-[pic/PIC] option

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: include/clang/Basic/DiagnosticDriverKinds.td:340
+  "ignoring '%0' option as it cannot be used with "
+  "explicit use of -mabicalls and the N64 ABI">,
   InGroup;

Use the %select{optionA|optionB|..|optionZ}$NUM operator here like:

"cannot be used with the %select{explicit|implicit}1 usage of -mabicalls and 
the N64 ABI"

Which eliminates the need for two separate warnings.



Comment at: lib/Driver/ToolChains/Arch/Mips.cpp:242-250
+if (!ABICallsArg) {
+  D.Diag(diag::warn_drv_unsupported_pic_with_implicit_mabicalls)
+  << LastPICArg->getAsString(Args);
+  NonPIC = false;
+} else if (UseAbiCalls) {
+  D.Diag(diag::warn_drv_unsupported_pic_with_explicit_mabicalls)
+  << LastPICArg->getAsString(Args);

Then all of this can be simplified.


Repository:
  rL LLVM

https://reviews.llvm.org/D44684



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44684: [mips] Improve handling of -fno-[pic/PIC] option

[Simon Dardis via Phabricator via cfe-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

LGTM, with a touch up of the error message to match the others we have 
regarding -mabicalls. Some other minor nits inlined.




Comment at: include/clang/Basic/DiagnosticDriverKinds.td:340
+  "ignoring '%0' option as it cannot be used with "
+  "explicit use of -mabicalls and the N64 ABI">,
   InGroup;

abeserminji wrote:
> sdardis wrote:
> > Use the %select{optionA|optionB|..|optionZ}$NUM operator here like:
> > 
> > "cannot be used with the %select{explicit|implicit}1 usage of -mabicalls 
> > and the N64 ABI"
> > 
> > Which eliminates the need for two separate warnings.
> Thanks for the hint. I simplified it now.
Just saw the error messages above; so for style, you should reformat the 
message to be of the form "ignoring '%0' option as it cannt be used with 
%select{|implicit usage of}1-mabicalls and the N64 ABI".

This way all the error messages are consistent. 



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:1024
+mips::getMipsCPUAndABI(Args, Triple, CPUName, ABIName);
+// For n64 ABI, PIC is always true, except in case when -mno-abicalls
+// option is used. And we exit at next check if that's true,

Nit: "When targeting the N64 ABI, PIC is the default, except in the case when 
the -mno-abicalls option is used."



Comment at: test/Driver/mips-abicalls-warning.c:2
 // REQUIRES: mips-registered-target
-// RUN: %clang -### -c -target mips64-mti-elf -fno-PIC -mabicalls %s 2>&1 | 
FileCheck %s
-// CHECK: warning: ignoring '-mabicalls' option as it cannot be used with non 
position-independent code and the N64 ABI
+// RUN: %clang -### -c -target mips64-mti-elf -fno-pic %s 2>&1 | FileCheck 
-check-prefix=CHECK-PIC1-IMPLICIT %s
+// CHECK-PIC1-IMPLICIT: warning: ignoring '-fno-pic' option as it cannot be 
used with implicit use of -mabicalls and the N64 ABI

Add some checks for -fno-pie,  -fno-PIE.


https://reviews.llvm.org/D44684



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41968: [libunwind][MIPS] Support MIPS floating-point registers for hard-float ABIs.

[Simon Dardis via Phabricator via cfe-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: chrib.

Sorry for the delay.

LGTM.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D41968



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

LGTM.


https://reviews.llvm.org/D38110



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35624: Removal of microMIPS64R6

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: lib/Basic/Targets/Mips.cpp:209
 bool MipsTargetInfo::validateTarget(DiagnosticsEngine &Diags) const {
+  // microMIPS64R6 backend is removed
+  if ((getTriple().getArch() == llvm::Triple::mips64 ||

"was removed."


https://reviews.llvm.org/D35624



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: include/__libunwind_config.h:73
+#  elif defined(_ABIN32) && defined(__mips_soft_float)
+#define _LIBUNWIND_TARGET_MIPS_NEWABI 1
+#define _LIBUNWIND_CONTEXT_SIZE 35

compnerd wrote:
> Minor nit: I prefer either `NABI` or `NEW_ABI`.
Normally mips documentation/source code either spells out n32/n64 when 
referring to the 64 bit abis or calls it a variation of NewABI/newabi/NEWABI 
(all one word).



Comment at: src/Registers.hpp:2252-2253
+#if defined(_LIBUNWIND_TARGET_MIPS_NEWABI)
+/// Registers_mips_newabi holds the register state of a thread in a NEWABI
+/// MIPS process including both the N32 and N64 ABIs.
+class _LIBUNWIND_HIDDEN Registers_mips_newabi {

Nit on the wording:

Registers_mips_newabi holds the register state of a thread in a NEWABI MIPS 
process including both the N32 and N64 ABIs. ->

Registers_mips_newabi holds the register state of a thread in a MIPS process 
using NEWABI (the N32 or N64 ABIs).


https://reviews.llvm.org/D39074



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36208: [mips] Enable target-specific attributes for MIPS64

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Some comments:

Currently there is no support in the backend for the interrupt attribute for 
mips64 / using N32 & N64 abis, it will give a fatal error. Previously the 
backend lacked support for the static relocation model which is an expected 
requirement for interrupt handlers.

microMIPS32r(3|6) is only supported for the O32 ABI, as microMIPS64R6 is 
deprecated and going to be removed. There is no support for the published 
microMIPS64R3 ISA.


Repository:
  rL LLVM

https://reviews.llvm.org/D36208



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36208: [mips] Enable target-specific attributes for MIPS64

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

I think for the interrupt attribute, it should be an error. Currently it's an 
implementation detail that it errors out in the backend but in principal it can 
be supported (I haven't gotten around to addressing it.)

For the micromips attribute, I believe it should be an error. My rational there 
is that the user has requested something that the compiler cannot perform.


Repository:
  rL LLVM

https://reviews.llvm.org/D36208



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36208: [mips] Enable target-specific attributes for MIPS64

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

@aaron.ballman I missed your first comments when I'd submitted mine.

In https://reviews.llvm.org/D36208#828957, @aaron.ballman wrote:

> In https://reviews.llvm.org/D36208#828955, @sdardis wrote:
>
> > I think for the interrupt attribute, it should be an error. Currently it's 
> > an implementation detail that it errors out in the backend but in principal 
> > it can be supported (I haven't gotten around to addressing it.)
>
>
> For the interrupt attribute, I think it should behave the same as the other 
> target-specific interrupt attributes unless there's a very good reason to be 
> inconsistent.


My primary thought there was that the backend will error out with "LLVM ERROR: 
"interrupt" attribute is only supported for the O32 ABI on MIPS32R2+ at the 
present time." if the attribute is seen anywhere by the backend for MIPS64. 
Having clang detect this case and being able to point out where in the code 
being compiled the attribute exists is better than having LLVM declaring 
there's an error somewhere and giving up. It is semantically critical as 
interrupt handlers need specific prologue+epilog sequences. I don't object to 
preserving the current behaviour of warning + ignoring it though.

> 
> 
>> For the micromips attribute, I believe it should be an error. My rational 
>> there is that the user has requested something that the compiler cannot 
>> perform.
> 
> That's not really sufficient rationale, because the same logic can be said of 
> any ignored attribute. As best I can tell from the docs on this attribute, 
> this controls code generation to turn on or off micromips code generation, 
> which sounds like the code otherwise generated may still work just fine. 
> e.g., `void f(void) __attribute__((micromips)) {}` -- if you compile that 
> code on a non-MIPS target, why should it produce an error instead of merely 
> warning the user that the attribute is ignored?

I stand corrected, we should just warn + ignore the micromips attribute for 
MIPS64 then.


Repository:
  rL LLVM

https://reviews.llvm.org/D36208



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Ping.


https://reviews.llvm.org/D35917



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35914: [mips] Add support -m(no-)embedded-data option

[Simon Dardis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL309935: [mips] Add support -m(no-)embedded-data option 
(authored by sdardis).

Repository:
  rL LLVM

https://reviews.llvm.org/D35914

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/test/Driver/mips-features.c


Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -1484,7 +1484,9 @@
 Arg *LocalSData = Args.getLastArg(options::OPT_mlocal_sdata,
   options::OPT_mno_local_sdata);
 Arg *ExternSData = Args.getLastArg(options::OPT_mextern_sdata,
-  options::OPT_mno_extern_sdata);
+   options::OPT_mno_extern_sdata);
+Arg *EmbeddedData = Args.getLastArg(options::OPT_membedded_data,
+options::OPT_mno_embedded_data);
 if (LocalSData) {
   CmdArgs.push_back("-mllvm");
   if (LocalSData->getOption().matches(options::OPT_mlocal_sdata)) {
@@ -1504,6 +1506,17 @@
   }
   ExternSData->claim();
 }
+
+if (EmbeddedData) {
+  CmdArgs.push_back("-mllvm");
+  if (EmbeddedData->getOption().matches(options::OPT_membedded_data)) {
+CmdArgs.push_back("-membedded-data=1");
+  } else {
+CmdArgs.push_back("-membedded-data=0");
+  }
+  EmbeddedData->claim();
+}
+
   } else if ((!ABICalls || (!NoABICalls && ABICalls)) && WantGPOpt)
 D.Diag(diag::warn_drv_unsupported_gpopt) << (ABICalls ? 0 : 1);
 
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -2051,6 +2051,12 @@
 def mno_extern_sdata : Flag<["-"], "mno-extern-sdata">, Group,
   HelpText<"Do not assume that externally defined data is in the small data if"
" it meets the -G  threshold (MIPS)">;
+def membedded_data : Flag<["-"], "membedded-data">, Group,
+  HelpText<"Place constants in the .rodata section instead of the .sdata "
+   "section even if they meet the -G  threshold (MIPS)">;
+def mno_embedded_data : Flag<["-"], "mno-embedded-data">, Group,
+  HelpText<"Do not place constants in the .rodata section instead of the "
+   ".sdata if they meet the -G  threshold (MIPS)">;
 def mnan_EQ : Joined<["-"], "mnan=">, Group;
 def mabicalls : Flag<["-"], "mabicalls">, Group,
   HelpText<"Enable SVR4-style position-independent code (Mips only)">;
Index: cfe/trunk/test/Driver/mips-features.c
===
--- cfe/trunk/test/Driver/mips-features.c
+++ cfe/trunk/test/Driver/mips-features.c
@@ -65,6 +65,21 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MEXTERNSDATADEF %s
 // CHECK-MEXTERNSDATADEF-NOT: "-mllvm" "-mextern-sdata"
 //
+// -mno-abicalls -mgpopt -membedded-data
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mgpopt 
-mno-embedded-data -membedded-data 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MEMBEDDEDDATA %s
+// CHECK-MEMBEDDEDDATA: "-mllvm" "-membedded-data=1"
+//
+// -mno-abicalls -mgpopt -mno-embedded-data
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mgpopt 
-membedded-data -mno-embedded-data 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MNOEMBEDDEDDATA %s
+// CHECK-MNOEMBEDDEDDATA: "-mllvm" "-membedded-data=0"
+//
+// -mno-abicalls -mgpopt
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MEMBEDDEDDATADEF %s
+// CHECK-MEMBEDDEDDATADEF-NOT: "-mllvm" "-membedded-data"
+//
 // -mips16
 // RUN: %clang -target mips-linux-gnu -### -c %s \
 // RUN: -mno-mips16 -mips16 2>&1 \


Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -1484,7 +1484,9 @@
 Arg *LocalSData = Args.getLastArg(options::OPT_mlocal_sdata,
   options::OPT_mno_local_sdata);
 Arg *ExternSData = Args.getLastArg(options::OPT_mextern_sdata,
-  options::OPT_mno_extern_sdata);
+   options::OPT_mno_extern_sdata);
+Arg *EmbeddedData = Args.getLastArg(options::OPT_membedded_data,
+options::OPT_mno_embedded_data);
 if (LocalSData) {
   CmdArgs.push_back("-mllvm");
   if (LocalSData->getOption().matches(options::OPT_mlocal_sdata)) {
@@ -1504,6 +1506,17 @@
   }
   ExternSData->claim();
 }
+
+if (EmbeddedData) {
+  CmdArgs.push_back("-mllvm");
+  if (EmbeddedData->getOption().matches(

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

[Simon Dardis via Phabricator via cfe-commits]

sdardis updated this revision to Diff 109545.
sdardis marked an inline comment as done.
sdardis added a comment.

Address review comment.


https://reviews.llvm.org/D35917

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/TargetInfo.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/mips-uninit-const-in-ro.c

Index: test/CodeGen/mips-uninit-const-in-ro.c
===
--- /dev/null
+++ test/CodeGen/mips-uninit-const-in-ro.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple mips-mti--elf -emit-llvm -mrelocation-model static \
+// RUN:-target-feature +noabicalls -mllvm -mgpopt -mllvm \
+// RUN:-membedded-data=1 -muninit-const-in-rodata -o - %s | \
+// RUN:   FileCheck %s
+
+// Test that -muninit-const-in-rodata places constant uninitialized structures
+// in the .rodata section rather than the commeon section.
+
+// CHECK: @a = global [8 x i32] zeroinitializer, section "rodata", align 4
+const int a[8];
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -950,6 +950,8 @@
 
   Opts.Backchain = Args.hasArg(OPT_mbackchain);
 
+  Opts.UInitCstDataInROData = Args.hasArg(OPT_muninit_const_in_rodata);
+
   Opts.EmitCheckPathComponentsToStrip = getLastArgIntValue(
   Args, OPT_fsanitize_undefined_strip_path_components_EQ, 0, Diags);
 
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1515,6 +1515,14 @@
 CmdArgs.push_back("-membedded-data=0");
   }
   EmbeddedData->claim();
+
+  if (Arg *A = Args.getLastArg(options::OPT_muninit_const_in_rodata,
+   options::OPT_mno_uninit_const_in_rodata)) {
+if (A->getOption().matches(options::OPT_muninit_const_in_rodata)) {
+  CmdArgs.push_back("-muninit-const-in-rodata");
+  A->claim();
+}
+  }
 }
 
   } else if ((!ABICalls || (!NoABICalls && ABICalls)) && WantGPOpt)
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -6656,6 +6656,20 @@
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &CGM,
ForDefinition_t IsForDefinition) const override {
+
+if (const VarDecl *VD = dyn_cast_or_null(D)) {
+  if (CGM.getCodeGenOpts().UInitCstDataInROData &&
+  VD->getType().isConstQualified() && !VD->hasInit()) {
+llvm::GlobalVariable *GVar = dyn_cast_or_null(GV);
+if (GVar && !GVar->hasSection()) {
+  GVar->setLinkage(llvm::GlobalValue::ExternalLinkage);
+  GVar->setSection("rodata");
+}
+  }
+
+  return;
+}
+
 const FunctionDecl *FD = dyn_cast_or_null(D);
 if (!FD) return;
 llvm::Function *Fn = cast(GV);
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -101,6 +101,8 @@
 CODEGENOPT(MergeFunctions, 1, 0) ///< Set when -fmerge-functions is enabled.
 CODEGENOPT(MSVolatile, 1, 0) ///< Set when /volatile:ms is enabled.
 CODEGENOPT(NoCommon  , 1, 0) ///< Set when -fno-common or C++ is enabled.
+CODEGENOPT(UInitCstDataInROData, 1, 0) ///< Set when -mgpopt & -membedded-data
+   ///< & -muinit-const-in-rodata is set
 CODEGENOPT(NoDwarfDirectoryAsm , 1, 0) ///< Set when -fno-dwarf-directory-asm is
///< enabled.
 CODEGENOPT(NoExecStack   , 1, 0) ///< Set when -Wa,--noexecstack is enabled.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -2057,6 +2057,14 @@
 def mno_embedded_data : Flag<["-"], "mno-embedded-data">, Group,
   HelpText<"Do not place constants in the .rodata section instead of the "
".sdata if they meet the -G  threshold (MIPS)">;
+def muninit_const_in_rodata : Flag<["-"], "muninit-const-in-rodata">,
+  Group, Flags<[DriverOption,CC1Option]>, HelpText<"Place "
+  "uninitialized constants in the read-only data section instead of"
+  " the common section (MIPS)">;
+def mno_uninit_const_in_rodata : Flag<["-"], "mno-uninit-const-in-rodata">,
+  Group, HelpText<"Do not place uninitialized constants in the "
+   "read-only data section instead of the common"
+   " section (MIPS)">;
 def mnan_EQ : Joined<["-"], "mnan=">, Group;
 def mabicalls : 

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:1523
+  CmdArgs.push_back("-muninit-const-in-rodata");
+  A->claim();
+}

atanasyan wrote:
> What's happened if the `-muninit-const-in-rodata` is used without 
> `-membedded-data`? Will the compiler shows a warning about unused command 
> line option?
Yes, it warns that it's unused.


https://reviews.llvm.org/D35917



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

[Simon Dardis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL309940: [mips] Implement -muninit-const-in-rodata (authored 
by sdardis).

Repository:
  rL LLVM

https://reviews.llvm.org/D35917

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/lib/CodeGen/TargetInfo.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/mips-uninit-const-in-ro.c

Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -1515,6 +1515,14 @@
 CmdArgs.push_back("-membedded-data=0");
   }
   EmbeddedData->claim();
+
+  if (Arg *A = Args.getLastArg(options::OPT_muninit_const_in_rodata,
+   options::OPT_mno_uninit_const_in_rodata)) {
+if (A->getOption().matches(options::OPT_muninit_const_in_rodata)) {
+  CmdArgs.push_back("-muninit-const-in-rodata");
+  A->claim();
+}
+  }
 }
 
   } else if ((!ABICalls || (!NoABICalls && ABICalls)) && WantGPOpt)
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -950,6 +950,8 @@
 
   Opts.Backchain = Args.hasArg(OPT_mbackchain);
 
+  Opts.UInitCstDataInROData = Args.hasArg(OPT_muninit_const_in_rodata);
+
   Opts.EmitCheckPathComponentsToStrip = getLastArgIntValue(
   Args, OPT_fsanitize_undefined_strip_path_components_EQ, 0, Diags);
 
Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -6656,6 +6656,20 @@
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &CGM,
ForDefinition_t IsForDefinition) const override {
+
+if (const VarDecl *VD = dyn_cast_or_null(D)) {
+  if (CGM.getCodeGenOpts().UInitCstDataInROData &&
+  VD->getType().isConstQualified() && !VD->hasInit()) {
+llvm::GlobalVariable *GVar = dyn_cast_or_null(GV);
+if (GVar && !GVar->hasSection()) {
+  GVar->setLinkage(llvm::GlobalValue::ExternalLinkage);
+  GVar->setSection("rodata");
+}
+  }
+
+  return;
+}
+
 const FunctionDecl *FD = dyn_cast_or_null(D);
 if (!FD) return;
 llvm::Function *Fn = cast(GV);
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -2057,6 +2057,14 @@
 def mno_embedded_data : Flag<["-"], "mno-embedded-data">, Group,
   HelpText<"Do not place constants in the .rodata section instead of the "
".sdata if they meet the -G  threshold (MIPS)">;
+def muninit_const_in_rodata : Flag<["-"], "muninit-const-in-rodata">,
+  Group, Flags<[DriverOption,CC1Option]>, HelpText<"Place "
+  "uninitialized constants in the read-only data section instead of"
+  " the common section (MIPS)">;
+def mno_uninit_const_in_rodata : Flag<["-"], "mno-uninit-const-in-rodata">,
+  Group, HelpText<"Do not place uninitialized constants in the "
+   "read-only data section instead of the common"
+   " section (MIPS)">;
 def mnan_EQ : Joined<["-"], "mnan=">, Group;
 def mabicalls : Flag<["-"], "mabicalls">, Group,
   HelpText<"Enable SVR4-style position-independent code (Mips only)">;
Index: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
===
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def
@@ -101,6 +101,8 @@
 CODEGENOPT(MergeFunctions, 1, 0) ///< Set when -fmerge-functions is enabled.
 CODEGENOPT(MSVolatile, 1, 0) ///< Set when /volatile:ms is enabled.
 CODEGENOPT(NoCommon  , 1, 0) ///< Set when -fno-common or C++ is enabled.
+CODEGENOPT(UInitCstDataInROData, 1, 0) ///< Set when -mgpopt & -membedded-data
+   ///< & -muinit-const-in-rodata is set
 CODEGENOPT(NoDwarfDirectoryAsm , 1, 0) ///< Set when -fno-dwarf-directory-asm is
///< enabled.
 CODEGENOPT(NoExecStack   , 1, 0) ///< Set when -Wa,--noexecstack is enabled.
Index: cfe/trunk/test/CodeGen/mips-uninit-const-in-ro.c
===
--- cfe/trunk/test/CodeGen/mips-uninit-const-in-ro.c
+++ cfe/trunk/test/CodeGen/mips-uninit-const-in-ro.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple mips-mti--elf -emit-llvm -mrelocatio

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Thanks for the reviews of all of these options.


Repository:
  rL LLVM

https://reviews.llvm.org/D35917



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

It's required for feature parity with GCC.

-fno-common will place globals into the bss section which uses memory at 
runtime. The -membedded-data and -muninit-const-in-rodata options are for 
reducing RAM usage in some embedded environments.


Repository:
  rL LLVM

https://reviews.llvm.org/D35917



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: cfe/trunk/lib/CodeGen/TargetInfo.cpp:
+  GVar->setLinkage(llvm::GlobalValue::ExternalLinkage);
+  GVar->setSection("rodata");
+}

efriedma wrote:
> Also, this change is clearly unacceptable; among other things, messing with 
> the linkage of static local variables is likely to lead to link errors.
I will fix that. 


Repository:
  rL LLVM

https://reviews.llvm.org/D35917



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

In https://reviews.llvm.org/D35917#830898, @joerg wrote:

> I don't see any reason why zero-initialised constants should be emitted in 
> BSS. I know that GCC does that and I just fixed bugs in that because created 
> wrong section flags for it. So yes, I'd prefer to revert this and fix the 
> real problem.


Will do.


Repository:
  rL LLVM

https://reviews.llvm.org/D35917



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

https://reviews.llvm.org/rL309978 for the revert.


Repository:
  rL LLVM

https://reviews.llvm.org/D35917



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36315: [mips] Support implicit gpopt with N64 when using -fno-pic

[Simon Dardis via Phabricator via cfe-commits]

sdardis created this revision.
Herald added a subscriber: arichardson.

As clang defaults to -mno-abicalls when using -fno-pic for N64, implicitly use 
-mgpopt in that case.


https://reviews.llvm.org/D36315

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/mips-features.c


Index: test/Driver/mips-features.c
===
--- test/Driver/mips-features.c
+++ test/Driver/mips-features.c
@@ -80,6 +80,24 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MEMBEDDEDDATADEF %s
 // CHECK-MEMBEDDEDDATADEF-NOT: "-mllvm" "-membedded-data"
 //
+// MIPS64 + N64: -fno-pic -> -mno-abicalls -mgpopt
+// RUN: %clang -target mips64-mti-elf -mabi=64 -### -c %s -fno-pic 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-N64-GPOPT %s
+// CHECK-N64-GPOPT: "-target-feature" "+noabicalls"
+// CHECK-N64-GPOPT: "-mllvm" "-mgpopt"
+//
+// MIPS64 + N64: -fno-pic -mno-gpopt
+// RUN: %clang -target mips64-mti-elf -mabi=64 -### -c %s -fno-pic -mno-gpopt 
2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-N64-MNO-GPOPT %s
+// CHECK-N64-MNO-GPOPT: "-target-feature" "+noabicalls"
+// CHECK-N64-MNO-GPOPT-NOT: "-mllvm" "-mgpopt"
+//
+// MIPS64 + N64: -mgpopt (-fpic is implicit)
+// RUN: %clang -target mips64-mti-linux-gnu -mabi=64 -### -c %s -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-N64-PIC-GPOPT %s
+// CHECK-N64-PIC-GPOPT-NOT: "-mllvm" "-mgpopt"
+// CHECK-N64-PIC-GPOPT: ignoring '-mgpopt' option as it cannot be used with 
the implicit usage of -mabicalls
+//
 // -mips16
 // RUN: %clang -target mips-linux-gnu -### -c %s \
 // RUN: -mno-mips16 -mips16 2>&1 \
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1473,8 +1473,21 @@
   // NOTE: We need a warning here or in the backend to warn when -mgpopt is
   //   passed explicitly when compiling something with -mabicalls
   //   (implictly) in affect. Currently the warning is in the backend.
+  //
+  // When the ABI in use is  N64, we also need to determine the PIC mode that
+  // is in use, as -fno-pic for N64 implies -mno-abicalls.
   bool NoABICalls =
   ABICalls && ABICalls->getOption().matches(options::OPT_mno_abicalls);
+
+  llvm::Reloc::Model RelocationModel;
+  unsigned PICLevel;
+  bool IsPIE;
+  std::tie(RelocationModel, PICLevel, IsPIE) =
+  ParsePICArgs(getToolChain(), Args);
+
+  NoABICalls = NoABICalls ||
+   (RelocationModel == llvm::Reloc::Static && ABIName == "n64");
+
   bool WantGPOpt = GPOpt && GPOpt->getOption().matches(options::OPT_mgpopt);
   // We quietly ignore -mno-gpopt as the backend defaults to -mno-gpopt.
   if (NoABICalls && (!GPOpt || WantGPOpt)) {


Index: test/Driver/mips-features.c
===
--- test/Driver/mips-features.c
+++ test/Driver/mips-features.c
@@ -80,6 +80,24 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MEMBEDDEDDATADEF %s
 // CHECK-MEMBEDDEDDATADEF-NOT: "-mllvm" "-membedded-data"
 //
+// MIPS64 + N64: -fno-pic -> -mno-abicalls -mgpopt
+// RUN: %clang -target mips64-mti-elf -mabi=64 -### -c %s -fno-pic 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-N64-GPOPT %s
+// CHECK-N64-GPOPT: "-target-feature" "+noabicalls"
+// CHECK-N64-GPOPT: "-mllvm" "-mgpopt"
+//
+// MIPS64 + N64: -fno-pic -mno-gpopt
+// RUN: %clang -target mips64-mti-elf -mabi=64 -### -c %s -fno-pic -mno-gpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-N64-MNO-GPOPT %s
+// CHECK-N64-MNO-GPOPT: "-target-feature" "+noabicalls"
+// CHECK-N64-MNO-GPOPT-NOT: "-mllvm" "-mgpopt"
+//
+// MIPS64 + N64: -mgpopt (-fpic is implicit)
+// RUN: %clang -target mips64-mti-linux-gnu -mabi=64 -### -c %s -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-N64-PIC-GPOPT %s
+// CHECK-N64-PIC-GPOPT-NOT: "-mllvm" "-mgpopt"
+// CHECK-N64-PIC-GPOPT: ignoring '-mgpopt' option as it cannot be used with the implicit usage of -mabicalls
+//
 // -mips16
 // RUN: %clang -target mips-linux-gnu -### -c %s \
 // RUN: -mno-mips16 -mips16 2>&1 \
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1473,8 +1473,21 @@
   // NOTE: We need a warning here or in the backend to warn when -mgpopt is
   //   passed explicitly when compiling something with -mabicalls
   //   (implictly) in affect. Currently the warning is in the backend.
+  //
+  // When the ABI in use is  N64, we also need to determine the PIC mode that
+  // is in use, as -fno-pic for N64 implies -mno-abicalls.
   bool NoABICalls =
   ABICalls && ABICalls->getOption().matches(options::OPT_mno_abicalls);
+
+  llvm::Reloc::Model RelocationModel;
+  unsigned PICLevel;
+  bool IsPIE;
+  std::tie(RelocationModel, PICLevel, IsPIE) =
+  ParsePICArgs(getToolChain(), Args);
+
+  NoABICalls = NoABICalls ||
+

[PATCH] D36551: [mips] Show warning in case of mixing -mlong-calls and -mabicalls options

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: include/clang/Basic/DiagnosticDriverKinds.td:296-297
+def warn_drv_unsupported_longcalls : Warning<
+  "ignoring '-mlong-calls' option as it cannot be used with "
+  "%select{|the implicit usage of }0-mabicalls">,
+  InGroup;

"as it is not currently supported with "

I think is better, as we currently don't support mixing the options but could 
do in the future.


Repository:
  rL LLVM

https://reviews.llvm.org/D36551



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36550: [mips] Notify user that `-mabicalls` is ignored on non-PIC N64 ABI

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Slight tweak to the summary:

> The -mabicalls option does not have a sense in case of non position 
> independent code on N64 ABI. After this change driver starts to show a 
> warning that -mabicalls is ignored in that case.

The -mabicalls option does not make sense in the case of non position 
independent code for the N64 ABI. After this change the driver shows a warning 
that -mabicalls is ignored in that case.

I think the major change in this patch is overly complex for what it should be, 
suggestion inline.




Comment at: include/clang/Basic/DiagnosticDriverKinds.td:297
+  "ignoring '-mabicalls' option as it cannot be used with "
+  "non position-independent code and N64 ABI">,
+  InGroup;

and the N64 ABI



Comment at: lib/Driver/ToolChains/Arch/Mips.cpp:233-253
+  Arg *ABICallsArg =
+  Args.getLastArg(options::OPT_mabicalls, options::OPT_mno_abicalls);
+  if (!ABICallsArg)
+NeedNoAbiCalls = DefNoAbiCalls;
+  else {
+if (ABICallsArg->getOption().matches(options::OPT_mno_abicalls))
+  NeedNoAbiCalls = true;

I think this logic can be simplified to:

  bool UseAbiCalls = false; 
   
  
  Arg *ABICallsArg =
  Args.getLastArg(options::OPT_mabicalls, options::OPT_mno_abicalls);   
   
  UseAbiCalls =
  !ABICallsArg || 
  (ABICallsArg && 
ABICallsArg->getOption().matches(options::OPT_mabicalls));  
 
  
  if (UseAbiCalls && IsN64 && NonPIC) {
D.Diag(diag::warn_drv_unsupported_abicalls);
   
UseAbiCalls = false;
   
  } 
   
  
  if (!UseAbiCalls)
Features.push_back("+noabicalls");  
   
  else
Features.push_back("-noabicalls"); 

As that way we immediately pick up the implicit case or explicit case of using 
-mabicalls and we can simply pick out the N64 && NonPic && UseAbiCalls case to 
warn and fix.

Although we don't need to add "-noabicalls" to the Features vector, the backend 
defaults to that, though the tests expect it.


Repository:
  rL LLVM

https://reviews.llvm.org/D36550



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36551: [mips] Show warning in case of mixing -mlong-calls and -mabicalls options

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: test/Driver/mips-longcalls-warning.c:1
+// REQUIRES: mips-registered-target
+// RUN: %clang -### -c -target mips-mti-elf -mlong-calls %s 2>&1 | FileCheck 
-check-prefix=IMPLICIT %s

Can you put this in test/Driver/mips-abicalls-warning.c from the other patch? 
I'd prefer to keep all the abicalls warnings together rather than separate 
tests based on the feature the conflict with.


Repository:
  rL LLVM

https://reviews.llvm.org/D36551



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36551: [mips] Show warning in case of mixing -mlong-calls and -mabicalls options

[Simon Dardis via Phabricator via cfe-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: test/Driver/mips-longcalls-warning.c:1
+// REQUIRES: mips-registered-target
+// RUN: %clang -### -c -target mips-mti-elf -mlong-calls %s 2>&1 | FileCheck 
-check-prefix=IMPLICIT %s

atanasyan wrote:
> sdardis wrote:
> > Can you put this in test/Driver/mips-abicalls-warning.c from the other 
> > patch? I'd prefer to keep all the abicalls warnings together rather than 
> > separate tests based on the feature the conflict with.
> > Can you put this in test/Driver/mips-abicalls-warning.c from the other 
> > patch?
> 
> I can, but in that case it's probably better to join both patches because 
> that patch loses its test.
> 
You can do it as an NFC change afterward both commits in that case.


Repository:
  rL LLVM

https://reviews.llvm.org/D36551



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36550: [mips] Notify user that `-mabicalls` is ignored on non-PIC N64 ABI

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Nit: my comment on line 297,  "non position-independent code and N64 ABI" 
should be "non position-independent code and the N64 ABI".

LGTM otherwise.


Repository:
  rL LLVM

https://reviews.llvm.org/D36550



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30465: [mips] Set the Int64Type / IntMaxType types correctly for OpenBSD/mips64

[Simon Dardis via Phabricator via cfe-commits]

sdardis closed this revision.
sdardis added a comment.

Closing this,  https://reviews.llvm.org/rL297098.


Repository:
  rL LLVM

https://reviews.llvm.org/D30465



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35624: Removal of microMIPS64R6

[Simon Dardis via Phabricator via cfe-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

One nit inlined, you can address it when committing.




Comment at: Basic/Targets/Mips.cpp:206
 bool MipsTargetInfo::validateTarget(DiagnosticsEngine &Diags) const {
+  if ((getTriple().getArch() == llvm::Triple::mips64 ||
+   getTriple().getArch() == llvm::Triple::mips64el) &&

Add a comment here stating that the microMIPS64R6 backend was removed.


Repository:
  rL LLVM

https://reviews.llvm.org/D35624



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35982: [mips] Introducing option -mabs=[legacy/2008]

[Simon Dardis via Phabricator via cfe-commits]

sdardis requested changes to this revision.
sdardis added a comment.
This revision now requires changes to proceed.

Follow @atanasyan 's suggestion and post a patch that renames that enum along 
with other changes.

One comment inlined.




Comment at: clang/Basic/DiagnosticDriverKinds.td:282-287
+def warn_target_unsupported_abslegacy : Warning<
+  "ignoring '-mabs=legacy' option because the '%0' architecture does not 
support it">,
+  InGroup;
+def warn_target_unsupported_abs2008 : Warning<
+  "ignoring '-mabs=2008' option because the '%0' architecture does not support 
it">,
+  InGroup;

These require tests. You should test that abs2008 is ignored pre-r2, legacy and 
abs2008 is accepted up to r5 and legacy is ignored for R6.


Repository:
  rL LLVM

https://reviews.llvm.org/D35982



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36315: [mips] Support implicit gpopt with N64 when using -fno-pic

[Simon Dardis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL310714: [mips] Support implicit gpopt with N64 when using 
-fno-pic (authored by sdardis).

Repository:
  rL LLVM

https://reviews.llvm.org/D36315

Files:
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/test/Driver/mips-features.c


Index: cfe/trunk/test/Driver/mips-features.c
===
--- cfe/trunk/test/Driver/mips-features.c
+++ cfe/trunk/test/Driver/mips-features.c
@@ -85,6 +85,24 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MEMBEDDEDDATADEF %s
 // CHECK-MEMBEDDEDDATADEF-NOT: "-mllvm" "-membedded-data"
 //
+// MIPS64 + N64: -fno-pic -> -mno-abicalls -mgpopt
+// RUN: %clang -target mips64-mti-elf -mabi=64 -### -c %s -fno-pic 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-N64-GPOPT %s
+// CHECK-N64-GPOPT: "-target-feature" "+noabicalls"
+// CHECK-N64-GPOPT: "-mllvm" "-mgpopt"
+//
+// MIPS64 + N64: -fno-pic -mno-gpopt
+// RUN: %clang -target mips64-mti-elf -mabi=64 -### -c %s -fno-pic -mno-gpopt 
2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-N64-MNO-GPOPT %s
+// CHECK-N64-MNO-GPOPT: "-target-feature" "+noabicalls"
+// CHECK-N64-MNO-GPOPT-NOT: "-mllvm" "-mgpopt"
+//
+// MIPS64 + N64: -mgpopt (-fpic is implicit)
+// RUN: %clang -target mips64-mti-linux-gnu -mabi=64 -### -c %s -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-N64-PIC-GPOPT %s
+// CHECK-N64-PIC-GPOPT-NOT: "-mllvm" "-mgpopt"
+// CHECK-N64-PIC-GPOPT: ignoring '-mgpopt' option as it cannot be used with 
the implicit usage of -mabicalls
+//
 // -mips16
 // RUN: %clang -target mips-linux-gnu -### -c %s \
 // RUN: -mno-mips16 -mips16 2>&1 \
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -1473,8 +1473,21 @@
   // NOTE: We need a warning here or in the backend to warn when -mgpopt is
   //   passed explicitly when compiling something with -mabicalls
   //   (implictly) in affect. Currently the warning is in the backend.
+  //
+  // When the ABI in use is  N64, we also need to determine the PIC mode that
+  // is in use, as -fno-pic for N64 implies -mno-abicalls.
   bool NoABICalls =
   ABICalls && ABICalls->getOption().matches(options::OPT_mno_abicalls);
+
+  llvm::Reloc::Model RelocationModel;
+  unsigned PICLevel;
+  bool IsPIE;
+  std::tie(RelocationModel, PICLevel, IsPIE) =
+  ParsePICArgs(getToolChain(), Args);
+
+  NoABICalls = NoABICalls ||
+   (RelocationModel == llvm::Reloc::Static && ABIName == "n64");
+
   bool WantGPOpt = GPOpt && GPOpt->getOption().matches(options::OPT_mgpopt);
   // We quietly ignore -mno-gpopt as the backend defaults to -mno-gpopt.
   if (NoABICalls && (!GPOpt || WantGPOpt)) {


Index: cfe/trunk/test/Driver/mips-features.c
===
--- cfe/trunk/test/Driver/mips-features.c
+++ cfe/trunk/test/Driver/mips-features.c
@@ -85,6 +85,24 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MEMBEDDEDDATADEF %s
 // CHECK-MEMBEDDEDDATADEF-NOT: "-mllvm" "-membedded-data"
 //
+// MIPS64 + N64: -fno-pic -> -mno-abicalls -mgpopt
+// RUN: %clang -target mips64-mti-elf -mabi=64 -### -c %s -fno-pic 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-N64-GPOPT %s
+// CHECK-N64-GPOPT: "-target-feature" "+noabicalls"
+// CHECK-N64-GPOPT: "-mllvm" "-mgpopt"
+//
+// MIPS64 + N64: -fno-pic -mno-gpopt
+// RUN: %clang -target mips64-mti-elf -mabi=64 -### -c %s -fno-pic -mno-gpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-N64-MNO-GPOPT %s
+// CHECK-N64-MNO-GPOPT: "-target-feature" "+noabicalls"
+// CHECK-N64-MNO-GPOPT-NOT: "-mllvm" "-mgpopt"
+//
+// MIPS64 + N64: -mgpopt (-fpic is implicit)
+// RUN: %clang -target mips64-mti-linux-gnu -mabi=64 -### -c %s -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-N64-PIC-GPOPT %s
+// CHECK-N64-PIC-GPOPT-NOT: "-mllvm" "-mgpopt"
+// CHECK-N64-PIC-GPOPT: ignoring '-mgpopt' option as it cannot be used with the implicit usage of -mabicalls
+//
 // -mips16
 // RUN: %clang -target mips-linux-gnu -### -c %s \
 // RUN: -mno-mips16 -mips16 2>&1 \
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -1473,8 +1473,21 @@
   // NOTE: We need a warning here or in the backend to warn when -mgpopt is
   //   passed explicitly when compiling something with -mabicalls
   //   (implictly) in affect. Currently the warning is in the backend.
+  //
+  // When the ABI in use is  N64, we also need to determine the PIC mode that
+  // is in use, as -fno-pic for N64 implies -mno-abicalls.
   bool NoABICalls =
   ABICalls && ABICalls->getOption().matches(options::OPT_mno_abicalls);
+
+  llvm::Reloc::Model RelocationModel;
+

[PATCH] D36315: [mips] Support implicit gpopt with N64 when using -fno-pic

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Thanks for the review and spotting this bug that lead me to finding the other 
bug.


Repository:
  rL LLVM

https://reviews.llvm.org/D36315



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36712: Emit section information for extern variables

[Simon Dardis via Phabricator via cfe-commits]

sdardis requested changes to this revision.
sdardis added a reviewer: kparzysz.
sdardis added a comment.
This revision now requires changes to proceed.

+kparzysz as Hexagon also makes use of the small data section.




Comment at: docs/LangRef.rst:629
+corresponding to the LLVM module, section information specified in the 
+declaration is retained in LLVM IR to enable OpenCL processes.
+

efriedma wrote:
> This doesn't really explain the part that matters. For LangRef, it doesn't 
> matter how the frontend decides to generate declarations (and it might be 
> different for frontends other than clang).  And OpenCL might motivate this, 
> but it has nothing to do with the semantics.
> 
> The key question here is what it actually means.  Is it a promise the that a 
> definition will exist for the given symbol in the given section?  If so, what 
> happens if the symbol is actually in a different section?  ("Undefined 
> behavior" is probably an acceptable answer, but it needs to be stated 
> explicitly.)
> to enable OpenCL processes.

for targets which make use of this section information. 

I would also work in a sentence that says that attaching section information to 
a external declaration is an assertion or promise that the definition is 
located in that section. If the definition is actually located in a different 
section the behaviour is undefined.

I'm favouring "undefined behaviour" in the error case as behaviour is so target 
and environment specific.


https://reviews.llvm.org/D36712



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36824: [mips] Rename getSupportedNanEncoding() to getIEEE754Standard() (NFC)

[Simon Dardis via Phabricator via cfe-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

LGTM with comment addressed.




Comment at: lib/Driver/ToolChains/Arch/Mips.cpp:328
+mips::IEEE754Standard mips::getIEEE754Standard(StringRef &CPU) {
+  // Strictly speaking, mips32r2 and mips64r2 are not in accordance to standard
+  // from 2008, it was first introduced in Release 3. However, other compilers 

"are not in accordance to standard from 2008, it was first introduced in 
Release 3."

This should read:

"do not conform to the IEEE754-2008 standard. Support for this standard was 
first introduced in Release 3."


Repository:
  rL LLVM

https://reviews.llvm.org/D36824



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35982: [mips] Introducing option -mabs=[legacy/2008]

[Simon Dardis via Phabricator via cfe-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

LGTM with a test for warnings. See test/Driver/mips-abicalls.c for reference.


Repository:
  rL LLVM

https://reviews.llvm.org/D35982



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36954: [Sema] Update release notes with details of implicit scalar to vector conversions

[Simon Dardis via Phabricator via cfe-commits]

sdardis created this revision.

Add notes on this to the C language section, along with the C++ section.


https://reviews.llvm.org/D36954

Files:
  docs/ReleaseNotes.rst


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -95,7 +95,41 @@
 C Language Changes in Clang
 ---
 
-- ...
+- Added near complete support for implicit scalar to vector conversion, a GNU
+  C/C++ language extension. With this extension, the following code is
+  considered valid:
+
+.. code-block:: c
+
+typedef unsigned v4i32 __attribute__((vector_size(16)));
+
+v4i32 foo(v4i32 a) {
+  // Here 5 is implicitly casted to an unsigned value and replicated into a
+  // vector with as many elements as 'a'.
+  return a + 5;
+}
+
+The implicit conversion of a scalar value to a vector value--in the context of
+a vector expression--occurs when:
+
+- The type of the vector is that of a ``__attribute__((vector_size(size)))``
+  vector, not an OpenCL ``__attribute__((ext_vector_type(size)))`` vector type.
+
+- The scalar value can be casted to that of the vector element's type without
+  the loss of precision based on the type of the scalar and the type of the
+  vector's elements.
+
+- For compile time constant values, the above rule is weakened to consider the
+  value of the scalar constant rather than the constant's type.
+
+- Floating point constants with precise integral representations are not
+  implicitly converted to integer values, this is for compatability with GCC.
+
+
+Currently the basic integer and floating point types with the following
+operators are supported: ``+``, ``/``, ``-``, ``*``, ``%``, ``>``, ``<``,
+``>=``, ``<=``, ``==``, ``!=``, ``&``, ``|``, ``^`` and the corresponding
+assignment operators where applicable.
 
 ...
 
@@ -107,6 +141,10 @@
 C++ Language Changes in Clang
 -
 
+- As mentioned in `C Language Changes in Clang`_, Clang's support for
+  implicit scalar to vector conversions also applies to C++. Additionally
+  the following operators are also supported: ``&&`` and ``||``.
+
 ...
 
 C++1z Feature Support


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -95,7 +95,41 @@
 C Language Changes in Clang
 ---
 
-- ...
+- Added near complete support for implicit scalar to vector conversion, a GNU
+  C/C++ language extension. With this extension, the following code is
+  considered valid:
+
+.. code-block:: c
+
+typedef unsigned v4i32 __attribute__((vector_size(16)));
+
+v4i32 foo(v4i32 a) {
+  // Here 5 is implicitly casted to an unsigned value and replicated into a
+  // vector with as many elements as 'a'.
+  return a + 5;
+}
+
+The implicit conversion of a scalar value to a vector value--in the context of
+a vector expression--occurs when:
+
+- The type of the vector is that of a ``__attribute__((vector_size(size)))``
+  vector, not an OpenCL ``__attribute__((ext_vector_type(size)))`` vector type.
+
+- The scalar value can be casted to that of the vector element's type without
+  the loss of precision based on the type of the scalar and the type of the
+  vector's elements.
+
+- For compile time constant values, the above rule is weakened to consider the
+  value of the scalar constant rather than the constant's type.
+
+- Floating point constants with precise integral representations are not
+  implicitly converted to integer values, this is for compatability with GCC.
+
+
+Currently the basic integer and floating point types with the following
+operators are supported: ``+``, ``/``, ``-``, ``*``, ``%``, ``>``, ``<``,
+``>=``, ``<=``, ``==``, ``!=``, ``&``, ``|``, ``^`` and the corresponding
+assignment operators where applicable.
 
 ...
 
@@ -107,6 +141,10 @@
 C++ Language Changes in Clang
 -
 
+- As mentioned in `C Language Changes in Clang`_, Clang's support for
+  implicit scalar to vector conversions also applies to C++. Additionally
+  the following operators are also supported: ``&&`` and ``||``.
+
 ...
 
 C++1z Feature Support
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36954: [Sema] Update release notes with details of implicit scalar to vector conversions

[Simon Dardis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311441: [Sema] Update release notes with details of implicit 
scalar to vector… (authored by sdardis).

Repository:
  rL LLVM

https://reviews.llvm.org/D36954

Files:
  cfe/branches/release_50/docs/ReleaseNotes.rst


Index: cfe/branches/release_50/docs/ReleaseNotes.rst
===
--- cfe/branches/release_50/docs/ReleaseNotes.rst
+++ cfe/branches/release_50/docs/ReleaseNotes.rst
@@ -116,7 +116,41 @@
 C Language Changes in Clang
 ---
 
-- ...
+- Added near complete support for implicit scalar to vector conversion, a GNU
+  C/C++ language extension. With this extension, the following code is
+  considered valid:
+
+.. code-block:: c
+
+typedef unsigned v4i32 __attribute__((vector_size(16)));
+
+v4i32 foo(v4i32 a) {
+  // Here 5 is implicitly casted to an unsigned value and replicated into a
+  // vector with as many elements as 'a'.
+  return a + 5;
+}
+
+The implicit conversion of a scalar value to a vector value--in the context of
+a vector expression--occurs when:
+
+- The type of the vector is that of a ``__attribute__((vector_size(size)))``
+  vector, not an OpenCL ``__attribute__((ext_vector_type(size)))`` vector type.
+
+- The scalar value can be casted to that of the vector element's type without
+  the loss of precision based on the type of the scalar and the type of the
+  vector's elements.
+
+- For compile time constant values, the above rule is weakened to consider the
+  value of the scalar constant rather than the constant's type.
+
+- Floating point constants with precise integral representations are not
+  implicitly converted to integer values, this is for compatability with GCC.
+
+
+Currently the basic integer and floating point types with the following
+operators are supported: ``+``, ``/``, ``-``, ``*``, ``%``, ``>``, ``<``,
+``>=``, ``<=``, ``==``, ``!=``, ``&``, ``|``, ``^`` and the corresponding
+assignment operators where applicable.
 
 ...
 
@@ -128,6 +162,10 @@
 C++ Language Changes in Clang
 -
 
+- As mentioned in `C Language Changes in Clang`_, Clang's support for
+  implicit scalar to vector conversions also applies to C++. Additionally
+  the following operators are also supported: ``&&`` and ``||``.
+
 ...
 
 C++1z Feature Support


Index: cfe/branches/release_50/docs/ReleaseNotes.rst
===
--- cfe/branches/release_50/docs/ReleaseNotes.rst
+++ cfe/branches/release_50/docs/ReleaseNotes.rst
@@ -116,7 +116,41 @@
 C Language Changes in Clang
 ---
 
-- ...
+- Added near complete support for implicit scalar to vector conversion, a GNU
+  C/C++ language extension. With this extension, the following code is
+  considered valid:
+
+.. code-block:: c
+
+typedef unsigned v4i32 __attribute__((vector_size(16)));
+
+v4i32 foo(v4i32 a) {
+  // Here 5 is implicitly casted to an unsigned value and replicated into a
+  // vector with as many elements as 'a'.
+  return a + 5;
+}
+
+The implicit conversion of a scalar value to a vector value--in the context of
+a vector expression--occurs when:
+
+- The type of the vector is that of a ``__attribute__((vector_size(size)))``
+  vector, not an OpenCL ``__attribute__((ext_vector_type(size)))`` vector type.
+
+- The scalar value can be casted to that of the vector element's type without
+  the loss of precision based on the type of the scalar and the type of the
+  vector's elements.
+
+- For compile time constant values, the above rule is weakened to consider the
+  value of the scalar constant rather than the constant's type.
+
+- Floating point constants with precise integral representations are not
+  implicitly converted to integer values, this is for compatability with GCC.
+
+
+Currently the basic integer and floating point types with the following
+operators are supported: ``+``, ``/``, ``-``, ``*``, ``%``, ``>``, ``<``,
+``>=``, ``<=``, ``==``, ``!=``, ``&``, ``|``, ``^`` and the corresponding
+assignment operators where applicable.
 
 ...
 
@@ -128,6 +162,10 @@
 C++ Language Changes in Clang
 -
 
+- As mentioned in `C Language Changes in Clang`_, Clang's support for
+  implicit scalar to vector conversions also applies to C++. Additionally
+  the following operators are also supported: ``&&`` and ``||``.
+
 ...
 
 C++1z Feature Support
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36954: [Sema] Update release notes with details of implicit scalar to vector conversions

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Thanks,
Simon


Repository:
  rL LLVM

https://reviews.llvm.org/D36954



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36712: Emit section information for extern variables

[Simon Dardis via Phabricator via cfe-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

I agree with @kparzysz here. If there is a mis-match between the declaration 
and definition, there are cases where undesirable behaviour (as such) will not 
occur depending on how the mismatch occurs.

One suggestion inline for the wording inline, the important part is making 
explicit

LGTM, anyone else have more comments?




Comment at: docs/LangRef.rst:582-584
+to be placed in and may have an optional explicit alignment specified. A 
+mismatch between section information in the variable declaration and its
+definition is undefined behavior. 

"If there is a mismatch between the explicit or inferred section information 
for the variable declaration and its definition the resulting behaviour is 
undefined."


https://reviews.llvm.org/D36712



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36712: Emit section information for extern variables

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

> One suggestion inline for the wording inline, the important part is making 
> explicit

The important part is making explicit that section information for a variable 
can be explicit or inferred.


https://reviews.llvm.org/D36712



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44381: [mips] Prevent PIC to be set to level 2

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:983
+  return std::make_tuple(llvm::Reloc::Static, 0U, false);
+// It's never PIC level 2 for mips.
+IsPICLevelTwo = false;

Can you reformulate this comment to say that MIPS does not use PIC level 2? 
Also, please add a note stating that even with -mxgot / -fPIC / multigot , MIPS 
does not use PIC level 2 unlike other architecutres for historical reasons.


Repository:
  rC Clang

https://reviews.llvm.org/D44381



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43509: [libcxx][cmake] Remove libatomic temporarily from CMAKE_REQUIRED_LIBRARIES when configuring

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Ping.


Repository:
  rCXX libc++

https://reviews.llvm.org/D43509



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44248: [clangd][cmake] Provide libatomic when there is no native support for 64bit atomics

[Simon Dardis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL329053: [clangd][cmake] Provide libatomic when there is no 
native support for 64bit… (authored by sdardis, committed by ).
Herald added subscribers: llvm-commits, MaskRay.

Changed prior to commit:
  https://reviews.llvm.org/D44248?vs=137555&id=140750#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44248

Files:
  clang-tools-extra/trunk/clangd/CMakeLists.txt


Index: clang-tools-extra/trunk/clangd/CMakeLists.txt
===
--- clang-tools-extra/trunk/clangd/CMakeLists.txt
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt
@@ -2,6 +2,11 @@
   Support
   )
 
+set(CLANGD_ATOMIC_LIB "")
+if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
+  list(APPEND CLANGD_ATOMIC_LIB "atomic")
+endif()
+
 add_clang_library(clangDaemon
   AST.cpp
   ClangdLSPServer.cpp
@@ -50,6 +55,7 @@
   clangToolingCore
   clangToolingRefactor
   ${LLVM_PTHREAD_LIB}
+  ${CLANGD_ATOMIC_LIB}
   )
 
 if( LLVM_LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )


Index: clang-tools-extra/trunk/clangd/CMakeLists.txt
===
--- clang-tools-extra/trunk/clangd/CMakeLists.txt
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt
@@ -2,6 +2,11 @@
   Support
   )
 
+set(CLANGD_ATOMIC_LIB "")
+if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
+  list(APPEND CLANGD_ATOMIC_LIB "atomic")
+endif()
+
 add_clang_library(clangDaemon
   AST.cpp
   ClangdLSPServer.cpp
@@ -50,6 +55,7 @@
   clangToolingCore
   clangToolingRefactor
   ${LLVM_PTHREAD_LIB}
+  ${CLANGD_ATOMIC_LIB}
   )
 
 if( LLVM_LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43509: [libcxx][cmake] Remove libatomic temporarily from CMAKE_REQUIRED_LIBRARIES when configuring

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Ping.


Repository:
  rCXX libc++

https://reviews.llvm.org/D43509



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43509: [libcxx][cmake] Remove libatomic temporarily from CMAKE_REQUIRED_LIBRARIES when configuring

[Simon Dardis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL329167: [libcxx][cmake] Remove libatomic temporarily from 
CMAKE_REQUIRED_LIBRARIES when… (authored by sdardis, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43509?vs=135044&id=140930#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43509

Files:
  libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake


Index: libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake
===
--- libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake
+++ libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake
@@ -31,7 +31,14 @@
   set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
 endfunction(check_cxx_atomics)
 
+# Perform the check for 64bit atomics without libatomic. It may have been
+# added to the required libraries during in the configuration of LLVM, which
+# would cause the check for CXX atomics without libatomic to incorrectly pass.
+set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
+list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES "atomic")
 check_cxx_atomics(LIBCXX_HAVE_CXX_ATOMICS_WITHOUT_LIB)
+set(CMAKE_REQUIRED_LIBRARIES ${OLD_CMAKE_REQUIRED_LIBRARIES})
+
 check_library_exists(atomic __atomic_fetch_add_8 "" LIBCXX_HAS_ATOMIC_LIB)
 # If not, check if the library exists, and atomics work with it.
 if(NOT LIBCXX_HAVE_CXX_ATOMICS_WITHOUT_LIB)


Index: libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake
===
--- libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake
+++ libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake
@@ -31,7 +31,14 @@
   set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
 endfunction(check_cxx_atomics)
 
+# Perform the check for 64bit atomics without libatomic. It may have been
+# added to the required libraries during in the configuration of LLVM, which
+# would cause the check for CXX atomics without libatomic to incorrectly pass.
+set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
+list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES "atomic")
 check_cxx_atomics(LIBCXX_HAVE_CXX_ATOMICS_WITHOUT_LIB)
+set(CMAKE_REQUIRED_LIBRARIES ${OLD_CMAKE_REQUIRED_LIBRARIES})
+
 check_library_exists(atomic __atomic_fetch_add_8 "" LIBCXX_HAS_ATOMIC_LIB)
 # If not, check if the library exists, and atomics work with it.
 if(NOT LIBCXX_HAVE_CXX_ATOMICS_WITHOUT_LIB)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43509: [libcxx][cmake] Remove libatomic temporarily from CMAKE_REQUIRED_LIBRARIES when configuring

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Thanks for the review.


Repository:
  rL LLVM

https://reviews.llvm.org/D43509



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44381: [mips] Prevent PIC to be set to level 2

[Simon Dardis via Phabricator via cfe-commits]

sdardis requested changes to this revision.
sdardis added a comment.
This revision now requires changes to proceed.

You should also update the description of the patch.


Repository:
  rC Clang

https://reviews.llvm.org/D44381



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40023: [RISCV] Implement ABI lowering

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

> Mips is the only other implementer of shouldSignExtUnsignedType but is 
> unaffected, as unlike classifyArgumentType, classifyReturnType will not 
> extend i32 values (@sdardis: is this a bug?).

I believe this is an oversight that is addressed in the backend. The MIPS64 
backend treats 'trunc i64 %x to i32' as '(SLL (EXTRACT_SUBREG GPR64:$src, 
sub_32), 0)', which is required to match MIPS32 arithmetic works on MIPS64.

I will take a longer look at this issue.

Thanks.


https://reviews.llvm.org/D40023



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

@compnerd, ping?


https://reviews.llvm.org/D38110



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

[Simon Dardis via Phabricator via cfe-commits]

sdardis updated this revision to Diff 81755.
sdardis marked an inline comment as done.
sdardis added a comment.

Addressed review comments, fixed assertion issue with expressions like scalar 
-= vector.


https://reviews.llvm.org/D25866

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/Sema/vector-cast.c
  test/Sema/vector-gcc-compat.c
  test/Sema/zvector.c
  test/SemaCXX/vector-no-lax.cpp

Index: test/SemaCXX/vector-no-lax.cpp
===
--- test/SemaCXX/vector-no-lax.cpp
+++ test/SemaCXX/vector-no-lax.cpp
@@ -4,6 +4,6 @@
 
 vSInt32 foo (vUInt32 a) {
   vSInt32 b = { 0, 0, 0, 0 };
-  b += a; // expected-error{{cannot convert between vector values}}
+  b += a; // expected-error{{cannot convert between vector type 'vUInt32' (vector of 4 'unsigned int' values) and vector type 'vSInt32' (vector of 4 'int' values) as implicit conversion would cause truncation}}
   return b;
 }
Index: test/Sema/zvector.c
===
--- test/Sema/zvector.c
+++ test/Sema/zvector.c
@@ -326,14 +326,14 @@
   bc = bc + sc2; // expected-error {{incompatible type}}
   bc = sc + bc2; // expected-error {{incompatible type}}
 
-  sc = sc + sc_scalar; // expected-error {{cannot convert}}
-  sc = sc + uc_scalar; // expected-error {{cannot convert}}
-  sc = sc_scalar + sc; // expected-error {{cannot convert}}
-  sc = uc_scalar + sc; // expected-error {{cannot convert}}
-  uc = uc + sc_scalar; // expected-error {{cannot convert}}
-  uc = uc + uc_scalar; // expected-error {{cannot convert}}
-  uc = sc_scalar + uc; // expected-error {{cannot convert}}
-  uc = uc_scalar + uc; // expected-error {{cannot convert}}
+  sc = sc + sc_scalar;
+  sc = sc + uc_scalar; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  sc = sc_scalar + sc;
+  sc = uc_scalar + sc; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  uc = uc + sc_scalar; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc = uc + uc_scalar;
+  uc = sc_scalar + uc; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc = uc_scalar + uc;
 
   ss = ss + ss2;
   us = us + us2;
@@ -368,10 +368,10 @@
   sc += sl2; // expected-error {{cannot convert}}
   sc += fd2; // expected-error {{cannot convert}}
 
-  sc += sc_scalar; // expected-error {{cannot convert}}
-  sc += uc_scalar; // expected-error {{cannot convert}}
-  uc += sc_scalar; // expected-error {{cannot convert}}
-  uc += uc_scalar; // expected-error {{cannot convert}}
+  sc += sc_scalar;
+  sc += uc_scalar; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  uc += sc_scalar; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc += uc_scalar;
 
   ss += ss2;
   us += us2;
Index: test/Sema/vector-gcc-compat.c
===
--- /dev/null
+++ test/Sema/vector-gcc-compat.c
@@ -0,0 +1,304 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -Weverything
+typedef long long v2i64 __attribute__((vector_size(16)));
+typedef int v2i32 __attribute__((vector_size(8)));
+typedef short v2i16 __attribute__((vector_size(4)));
+typedef char v2i8 __attribute__((vector_size(2)));
+
+typedef unsigned long long v2u64 __attribute__((vector_size(16)));
+typedef unsigned int v2u32 __attribute__((vector_size(8)));
+typedef unsigned short v2u16 __attribute__((vector_size(4)));
+typedef unsigned char v2u8 __attribute__((vector_size(2)));
+
+typedef float v4f32 __attribute__((vector_size(16)));
+typedef double v4f64 __attribute__((vector_size(32)));
+
+void arithmeticTest(void);
+void logicTest(void);
+void comparisonTest(void);
+void floatTestSignedType(char a, short b, int c, long long d);
+void floatTestUnsignedType(unsigned char a, unsigned short b, unsigned int c,
+   unsigned long long d);
+void floatTestConstant(void);
+void intTestType(char a, short b, int c, long long d);
+void intTestTypeUnsigned(unsigned char a, unsigned short b, unsigned int c,
+ unsigned long long d);
+void uintTestType(char a, short b, int c, long long d);
+void uintTestTypeUnsigned(unsigned char a, unsigned short b, unsigned int c,
+  unsigned long long d);
+void uintTestConstant(v2u64 v2u64_a, v2u32 v2u32_a, v2u16 v2u

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:8051
+  if (!LHSVecType) {
+assert(RHSVecType && "RHSVecType is not a vector!");
 if (!tryVectorConvertAndSplat(*this, (IsCompAssign ? nullptr : &LHS),

bruno wrote:
> `tryVectorConvertAndSplat` does more than plain scalar splat; it supports a 
> more general type of CK_IntegralCast, see the comment on one of your changes 
> to the tests below.
> 
> I suggest that instead of reusing this function, you should create another 
> one that only handles the cases we actually want to support for non-ext 
> vectors (i.e. for GCC compat). 
(ignore this, phabricator's web interface is acting up)



Comment at: lib/Sema/SemaExpr.cpp:8267
+  }
+
   // Otherwise, use the generic diagnostic.

bruno wrote:
> sdardis wrote:
> > bruno wrote:
> > > This change seems orthogonal to this patch. Can you make it a separated 
> > > patch with the changes from test/Sema/vector-cast.c?
> > This change is a necessary part of this patch and it can't be split out in 
> > sensible fashion.
> > 
> > For example, line 329 of test/Sema/zvector.c adds a scalar signed character 
> > to a vector of signed characters. With just this change we would report 
> > "cannot convert between scalar type 'signed char' and vector type '__vector 
> > signed char' (vector of 16 'signed char' values) as implicit conversion 
> > would cause truncation".
> > 
> > This is heavily misleading for C and C++ code as we don't perform implicit 
> > conversions and signed char could be implicitly converted to a vector of 
> > signed char without the loss of precision.
> > 
> > To make this diagnostic precise without performing implicit conversions 
> > requires determining cases where implicit conversion would cause truncation 
> > and reporting that failure reason, or defaulting to the generic diagnostic.
> > 
> > Keeping this change as part of this patch is cleaner and simpler as it 
> > covers both implicit conversions and more precise diagnostics.
> Can you double check whether we should support these GCC semantics for 
> getLangOpts().ZVector? If not, then this should be introduced in a separate 
> patch/commit.
See my comment above.



Comment at: lib/Sema/SemaExpr.cpp:8171
+return LHSType;
+}
   }

bruno wrote:
> It's not clear to me whether clang should support the GCC semantics when in 
> `getLangOpts().AltiVec || getLangOpts().ZVector`, do you?
> 
> You can remove the curly braces for the `if` and `else` here.
We should, GCC performs scalar to vector conversions regardless of what cpu 
features are available. If the target machine doesn't have vector support, GCC 
silently scalarizes the operation.

https://gcc.gnu.org/onlinedocs/gcc-6.2.0/gcc/Vector-Extensions.html#Vector-Extensions


https://reviews.llvm.org/D25866



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Ping.


https://reviews.llvm.org/D25866



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31082: [mips][msa] Range adjustment for ldi_b builtin function operand

[Simon Dardis via Phabricator via cfe-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

Can you add a test to test/CodeGen/builtins-mips-msa.c covering the new 
extended range for ldi.b ?

LGTM.


https://reviews.llvm.org/D31082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30465: [mips] Set the Int64Type / IntMaxType types correctly for OpenBSD/mips64

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Do you need me to commit this for you?


Repository:
  rL LLVM

https://reviews.llvm.org/D30465



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31667: [Sema] Extend GetSignedVectorType to deal with non ExtVector types

[Simon Dardis via Phabricator via cfe-commits]

sdardis created this revision.

This improves some error messages which would otherwise refer to
ext_vector_type types in contexts where there are no such types.

Factored out from https://reviews.llvm.org/D25866 at reviewer's request.


https://reviews.llvm.org/D31667

Files:
  include/clang/Sema/Sema.h
  lib/Sema/SemaExpr.cpp
  test/Sema/vector-ops.c

Index: test/Sema/vector-ops.c
===
--- test/Sema/vector-ops.c
+++ test/Sema/vector-ops.c
@@ -13,7 +13,7 @@
   (void)(~v2fa); // expected-error{{invalid argument type 'v2f' (vector of 2 'float' values) to unary}}
 
   // Comparison operators
-  v2ua = (v2ua==v2sa); // expected-warning{{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from 'int __attribute__((ext_vector_type(2)))' (vector of 2 'int' values)}}
+  v2ua = (v2ua==v2sa); // expected-warning{{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
   v2sa = (v2ua==v2sa);
   
   // Arrays
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -9711,24 +9711,45 @@
   return InvalidOperands(Loc, LHS, RHS);
 }
 
-
-// Return a signed type that is of identical size and number of elements.
-// For floating point vectors, return an integer type of identical size 
-// and number of elements.
-QualType Sema::GetSignedVectorType(QualType V) {
+// Return a signed ext_vector type that is of identical size and number of
+// elements. For floating point vectors, return an integer type of identical
+// size and number of elements. In the non ext_vector case, search from the
+// largest to the smallest to avoid cases where long long == long, where long
+// gets picked over long long.
+QualType Sema::GetSignedVectorType(QualType V, bool WantExtVector) {
   const VectorType *VTy = V->getAs();
   unsigned TypeSize = Context.getTypeSize(VTy->getElementType());
-  if (TypeSize == Context.getTypeSize(Context.CharTy))
-return Context.getExtVectorType(Context.CharTy, VTy->getNumElements());
-  else if (TypeSize == Context.getTypeSize(Context.ShortTy))
-return Context.getExtVectorType(Context.ShortTy, VTy->getNumElements());
-  else if (TypeSize == Context.getTypeSize(Context.IntTy))
-return Context.getExtVectorType(Context.IntTy, VTy->getNumElements());
+
+  if (WantExtVector) {
+if (TypeSize == Context.getTypeSize(Context.CharTy))
+  return Context.getExtVectorType(Context.CharTy, VTy->getNumElements());
+else if (TypeSize == Context.getTypeSize(Context.ShortTy))
+  return Context.getExtVectorType(Context.ShortTy, VTy->getNumElements());
+else if (TypeSize == Context.getTypeSize(Context.IntTy))
+  return Context.getExtVectorType(Context.IntTy, VTy->getNumElements());
+else if (TypeSize == Context.getTypeSize(Context.LongTy))
+  return Context.getExtVectorType(Context.LongTy, VTy->getNumElements());
+assert(TypeSize == Context.getTypeSize(Context.LongLongTy) &&
+   "Unhandled vector element size in vector compare");
+return Context.getExtVectorType(Context.LongLongTy, VTy->getNumElements());
+  }
+
+  if (TypeSize == Context.getTypeSize(Context.LongLongTy))
+return Context.getVectorType(Context.LongLongTy, VTy->getNumElements(),
+ VectorType::GenericVector);
   else if (TypeSize == Context.getTypeSize(Context.LongTy))
-return Context.getExtVectorType(Context.LongTy, VTy->getNumElements());
-  assert(TypeSize == Context.getTypeSize(Context.LongLongTy) &&
+return Context.getVectorType(Context.LongTy, VTy->getNumElements(),
+ VectorType::GenericVector);
+  else if (TypeSize == Context.getTypeSize(Context.IntTy))
+return Context.getVectorType(Context.IntTy, VTy->getNumElements(),
+ VectorType::GenericVector);
+  else if (TypeSize == Context.getTypeSize(Context.ShortTy))
+return Context.getVectorType(Context.ShortTy, VTy->getNumElements(),
+ VectorType::GenericVector);
+  assert(TypeSize == Context.getTypeSize(Context.CharTy) &&
  "Unhandled vector element size in vector compare");
-  return Context.getExtVectorType(Context.LongLongTy, VTy->getNumElements());
+  return Context.getVectorType(Context.CharTy, VTy->getNumElements(),
+   VectorType::GenericVector);
 }
 
 /// CheckVectorCompareOperands - vector comparisons are a clang extension that
@@ -9747,6 +9768,8 @@
 return vType;
 
   QualType LHSType = LHS.get()->getType();
+  bool isExtVectorType =
+  isa(vType->getAs()) || getLangOpts().OpenCL;
 
   // If AltiVec, the comparison results in a numeric type, i.e.
   // bool for C++, int for C
@@ -9775,9 +9798,9 @@
 assert (RHS.get()->getType()->hasFloatingRepresentation());

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

[Simon Dardis via Phabricator via cfe-commits]

sdardis updated this revision to Diff 94070.
sdardis added a comment.

Factored out the changes from https://reviews.llvm.org/D31337.


https://reviews.llvm.org/D25866

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaExpr.cpp
  test/Sema/vector-cast.c
  test/Sema/vector-g++-compat.cpp
  test/Sema/vector-gcc-compat.c
  test/Sema/zvector.c
  test/SemaCXX/vector-no-lax.cpp

Index: test/SemaCXX/vector-no-lax.cpp
===
--- test/SemaCXX/vector-no-lax.cpp
+++ test/SemaCXX/vector-no-lax.cpp
@@ -4,6 +4,6 @@
 
 vSInt32 foo (vUInt32 a) {
   vSInt32 b = { 0, 0, 0, 0 };
-  b += a; // expected-error{{cannot convert between vector values}}
+  b += a; // expected-error{{cannot convert between vector type 'vUInt32' (vector of 4 'unsigned int' values) and vector type 'vSInt32' (vector of 4 'int' values) as implicit conversion would cause truncation}}
   return b;
 }
Index: test/Sema/zvector.c
===
--- test/Sema/zvector.c
+++ test/Sema/zvector.c
@@ -326,14 +326,14 @@
   bc = bc + sc2; // expected-error {{incompatible type}}
   bc = sc + bc2; // expected-error {{incompatible type}}
 
-  sc = sc + sc_scalar; // expected-error {{cannot convert}}
-  sc = sc + uc_scalar; // expected-error {{cannot convert}}
-  sc = sc_scalar + sc; // expected-error {{cannot convert}}
-  sc = uc_scalar + sc; // expected-error {{cannot convert}}
-  uc = uc + sc_scalar; // expected-error {{cannot convert}}
-  uc = uc + uc_scalar; // expected-error {{cannot convert}}
-  uc = sc_scalar + uc; // expected-error {{cannot convert}}
-  uc = uc_scalar + uc; // expected-error {{cannot convert}}
+  sc = sc + sc_scalar;
+  sc = sc + uc_scalar; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  sc = sc_scalar + sc;
+  sc = uc_scalar + sc; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  uc = uc + sc_scalar; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc = uc + uc_scalar;
+  uc = sc_scalar + uc; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc = uc_scalar + uc;
 
   ss = ss + ss2;
   us = us + us2;
@@ -368,10 +368,10 @@
   sc += sl2; // expected-error {{cannot convert}}
   sc += fd2; // expected-error {{cannot convert}}
 
-  sc += sc_scalar; // expected-error {{cannot convert}}
-  sc += uc_scalar; // expected-error {{cannot convert}}
-  uc += sc_scalar; // expected-error {{cannot convert}}
-  uc += uc_scalar; // expected-error {{cannot convert}}
+  sc += sc_scalar;
+  sc += uc_scalar; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  uc += sc_scalar; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc += uc_scalar;
 
   ss += ss2;
   us += us2;
Index: test/Sema/vector-gcc-compat.c
===
--- /dev/null
+++ test/Sema/vector-gcc-compat.c
@@ -0,0 +1,335 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -Weverything
+
+// Test the compatibility of clang's vector extensions with gcc's vector
+// extensions for C. Notably &&, ||, ?: and ! are not available.
+typedef long long v2i64 __attribute__((vector_size(16)));
+typedef int v2i32 __attribute__((vector_size(8)));
+typedef short v2i16 __attribute__((vector_size(4)));
+typedef char v2i8 __attribute__((vector_size(2)));
+
+typedef unsigned long long v2u64 __attribute__((vector_size(16)));
+typedef unsigned int v2u32 __attribute__((vector_size(8)));
+typedef unsigned short v2u16 __attribute__((vector_size(4)));
+typedef unsigned char v2u8 __attribute__((vector_size(2)));
+
+typedef float v4f32 __attribute__((vector_size(16)));
+typedef double v2f64 __attribute__((vector_size(16)));
+typedef double v4f64 __attribute__((vector_size(32)));
+typedef int v4i32 __attribute((vector_size(16)));
+
+void arithmeticTest(void);
+void logicTest(void);
+void comparisonTest(void);
+void floatTestSignedType(char a, short b, int c, long long d);
+void floatTestUnsignedType(unsigned char a, unsigned short b, unsigned int c,
+   unsigned long long d);
+void floatTestConstant(void);
+void intTestType(char a, short b, int c, long long d);
+void intTestTypeUnsigned(unsigned char a, unsigned short b, unsigned int c,
+ unsigned long long d);
+void

[PATCH] D31667: [Sema] Extend GetSignedVectorType to deal with non ExtVector types

[Simon Dardis via Phabricator via cfe-commits]

sdardis updated this revision to Diff 94205.
sdardis marked 5 inline comments as done.
sdardis added a comment.

Addressed review comments, added more tests.


https://reviews.llvm.org/D31667

Files:
  lib/Sema/SemaExpr.cpp
  test/Sema/vector-ops.c

Index: test/Sema/vector-ops.c
===
--- test/Sema/vector-ops.c
+++ test/Sema/vector-ops.c
@@ -13,9 +13,9 @@
   (void)(~v2fa); // expected-error{{invalid argument type 'v2f' (vector of 2 'float' values) to unary}}
 
   // Comparison operators
-  v2ua = (v2ua==v2sa); // expected-warning{{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from 'int __attribute__((ext_vector_type(2)))' (vector of 2 'int' values)}}
+  v2ua = (v2ua==v2sa); // expected-warning{{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
   v2sa = (v2ua==v2sa);
-  
+
   // Arrays
   int array1[v2ua]; // expected-error{{size of array has non-integer type 'v2u' (vector of 2 'unsigned int' values)}}
   int array2[17];
@@ -26,4 +26,110 @@
   v2s *v2s_ptr;
   v2s_ptr = v2u_ptr; // expected-warning{{converts between pointers to integer types with different sign}}
 }
- 
+
+void testLogicalVecVec(v2u v2ua, v2s v2sa, v2f v2fa) {
+
+  // Logical operators
+  v2ua = v2ua && v2ua; // expected-warning {{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+  v2ua = v2ua || v2ua; // expected-warning {{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+
+  v2ua = v2sa && v2ua; // expected-warning {{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+  v2ua = v2sa || v2ua; // expected-warning {{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+
+  v2ua = v2ua && v2fa; // expected-warning {{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+  v2ua = v2ua || v2fa; // expected-warning {{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+
+  v2ua = v2sa && v2fa; // expected-warning {{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+  v2ua = v2sa || v2fa; // expected-warning {{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+
+  v2sa = v2sa && v2sa;
+  v2sa = v2sa || v2sa;
+
+  v2sa = v2ua && v2ua;
+  v2sa = v2ua || v2ua;
+
+  v2sa = v2sa && v2ua;
+  v2sa = v2sa || v2ua;
+
+  v2sa = v2sa && v2fa;
+  v2sa = v2sa || v2fa;
+
+  v2sa = v2ua && v2fa;
+  v2sa = v2ua || v2fa;
+
+  v2fa = v2fa && v2fa; // expected-warning {{incompatible vector types assigning to 'v2f' (vector of 2 'float' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+  v2fa = v2fa || v2fa; // expected-warning {{incompatible vector types assigning to 'v2f' (vector of 2 'float' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+
+  v2fa = v2sa && v2fa; // expected-warning {{incompatible vector types assigning to 'v2f' (vector of 2 'float' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+  v2fa = v2sa || v2fa; // expected-warning {{incompatible vector types assigning to 'v2f' (vector of 2 'float' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+
+  v2fa = v2ua && v2fa; // expected-warning {{incompatible vector types assigning to 'v2f' (vector of 2 'float' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+  v2fa = v2ua || v2fa; // expected-warning {{incompatible vector types assigning to 'v2f' (vector of 2 'float' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+
+  v2fa = v2ua && v2ua; // expected-warning {{incompatible vector types assigning to 'v2f' (vector of 2 'float' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+  v2fa = v2ua || v2ua; // expected-warning {{incompatible vector types assigning to 'v2f' (vector of 2 'float' values) from '__attribute__((

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

[Simon Dardis via Phabricator via cfe-commits]

sdardis marked an inline comment as done.
sdardis added a comment.

Thanks for sticking with this.

I've held off updating the diff until https://reviews.llvm.org/D31667 (not 
https://reviews.llvm.org/D31337 as previously posted) is finished. Responses 
inlined.




Comment at: lib/Sema/SemaExpr.cpp:8188
+  //type and then perform the rest of the checks here. GCC as of
+  //pre-release 7.0 does not accept this though.
+  if (VectorEltTy->isIntegralType(S.Context) &&

bruno wrote:
> Is this something that GCC is going to support at some point? Regardless of 
> GCC, do we want this behavior?
> Is this something that GCC is going to support at some point?

I've reread GCC's source for conversions of scalar to vectors types and it 
appears that GCC does attempt to convert constant real expressions to integers 
but it appears to only work for C++. It's a little odd.

> Regardless of GCC, do we want this behavior?

Given my oversight of the support of implicit safe conversions of floating 
point constants to integer types, I believe we should do this in general. I'll 
rework the comment to be more accurate and add the necessary conversion checks.



Comment at: lib/Sema/SemaExpr.cpp:10019
   isa(vType->getAs()) || getLangOpts().OpenCL;
+  if ((!getLangOpts().CPlusPlus && !getLangOpts().OpenCL) && !isExtVectorType)
+return InvalidVectorOperands(Loc, LHS, RHS);

bruno wrote:
> Why `!getLangOpts().CPlusPlus` is a requirement here?
GCC only supports the logical operators &&, ||, ! when compiling C++.

It's noted near the bottom half of this page: 
https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html



Comment at: test/Sema/vector-g++-compat.cpp:155
+   unsigned long long d) { // expected-warning {{'long 
long' is incompatible with C++98}}
+
+  v4f32 v4f32_a = {0.4f, 0.4f, 0.4f, 0.4f};

bruno wrote:
> Remove all these extra new lines after the function signature here, in the 
> functions below and in the other added test file for c++.
Will do.


https://reviews.llvm.org/D25866



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31667: [Sema] Extend GetSignedVectorType to deal with non ExtVector types

[Simon Dardis via Phabricator via cfe-commits]

sdardis marked an inline comment as done.
sdardis added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:9736
+  }
+
+  if (TypeSize == Context.getTypeSize(Context.LongLongTy))

ahatanak wrote:
> This isn't particularly urgent, but can we use ASTContext 
> ::getIntTypeForBitwidth here rather than calling getVectorType or 
> getExtVectorType in many places?
> 
Yeah, it appears we can. I'll commit the current version and post a refactoring 
patch.


https://reviews.llvm.org/D31667



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31667: [Sema] Extend GetSignedVectorType to deal with non ExtVector types

[Simon Dardis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL299641: [Sema] Extend GetSignedVectorType to deal with non 
ExtVector types (authored by sdardis).

Changed prior to commit:
  https://reviews.llvm.org/D31667?vs=94205&id=94344#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31667

Files:
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/Sema/vector-ops.c

Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -9711,24 +9711,45 @@
   return InvalidOperands(Loc, LHS, RHS);
 }
 
-
-// Return a signed type that is of identical size and number of elements.
-// For floating point vectors, return an integer type of identical size 
-// and number of elements.
+// Return a signed ext_vector_type that is of identical size and number of
+// elements. For floating point vectors, return an integer type of identical
+// size and number of elements. In the non ext_vector_type case, search from
+// the largest type to the smallest type to avoid cases where long long == long,
+// where long gets picked over long long.
 QualType Sema::GetSignedVectorType(QualType V) {
   const VectorType *VTy = V->getAs();
   unsigned TypeSize = Context.getTypeSize(VTy->getElementType());
-  if (TypeSize == Context.getTypeSize(Context.CharTy))
-return Context.getExtVectorType(Context.CharTy, VTy->getNumElements());
-  else if (TypeSize == Context.getTypeSize(Context.ShortTy))
-return Context.getExtVectorType(Context.ShortTy, VTy->getNumElements());
-  else if (TypeSize == Context.getTypeSize(Context.IntTy))
-return Context.getExtVectorType(Context.IntTy, VTy->getNumElements());
+
+  if (isa(VTy)) {
+if (TypeSize == Context.getTypeSize(Context.CharTy))
+  return Context.getExtVectorType(Context.CharTy, VTy->getNumElements());
+else if (TypeSize == Context.getTypeSize(Context.ShortTy))
+  return Context.getExtVectorType(Context.ShortTy, VTy->getNumElements());
+else if (TypeSize == Context.getTypeSize(Context.IntTy))
+  return Context.getExtVectorType(Context.IntTy, VTy->getNumElements());
+else if (TypeSize == Context.getTypeSize(Context.LongTy))
+  return Context.getExtVectorType(Context.LongTy, VTy->getNumElements());
+assert(TypeSize == Context.getTypeSize(Context.LongLongTy) &&
+   "Unhandled vector element size in vector compare");
+return Context.getExtVectorType(Context.LongLongTy, VTy->getNumElements());
+  }
+
+  if (TypeSize == Context.getTypeSize(Context.LongLongTy))
+return Context.getVectorType(Context.LongLongTy, VTy->getNumElements(),
+ VectorType::GenericVector);
   else if (TypeSize == Context.getTypeSize(Context.LongTy))
-return Context.getExtVectorType(Context.LongTy, VTy->getNumElements());
-  assert(TypeSize == Context.getTypeSize(Context.LongLongTy) &&
+return Context.getVectorType(Context.LongTy, VTy->getNumElements(),
+ VectorType::GenericVector);
+  else if (TypeSize == Context.getTypeSize(Context.IntTy))
+return Context.getVectorType(Context.IntTy, VTy->getNumElements(),
+ VectorType::GenericVector);
+  else if (TypeSize == Context.getTypeSize(Context.ShortTy))
+return Context.getVectorType(Context.ShortTy, VTy->getNumElements(),
+ VectorType::GenericVector);
+  assert(TypeSize == Context.getTypeSize(Context.CharTy) &&
  "Unhandled vector element size in vector compare");
-  return Context.getExtVectorType(Context.LongLongTy, VTy->getNumElements());
+  return Context.getVectorType(Context.CharTy, VTy->getNumElements(),
+   VectorType::GenericVector);
 }
 
 /// CheckVectorCompareOperands - vector comparisons are a clang extension that
@@ -9775,7 +9796,7 @@
 assert (RHS.get()->getType()->hasFloatingRepresentation());
 CheckFloatComparison(Loc, LHS.get(), RHS.get());
   }
-  
+
   // Return a signed type for the vector.
   return GetSignedVectorType(vType);
 }
@@ -9792,7 +9813,7 @@
   if (getLangOpts().OpenCL && getLangOpts().OpenCLVersion < 120 &&
   vType->hasFloatingRepresentation())
 return InvalidOperands(Loc, LHS, RHS);
-  
+
   return GetSignedVectorType(LHS.get()->getType());
 }
 
Index: cfe/trunk/test/Sema/vector-ops.c
===
--- cfe/trunk/test/Sema/vector-ops.c
+++ cfe/trunk/test/Sema/vector-ops.c
@@ -13,9 +13,9 @@
   (void)(~v2fa); // expected-error{{invalid argument type 'v2f' (vector of 2 'float' values) to unary}}
 
   // Comparison operators
-  v2ua = (v2ua==v2sa); // expected-warning{{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from 'int __attribute__((ext_vector_type(2)))' (vector of 2 'int' values)}}
+  v2ua = (v2ua==v2sa); // expected-warning{{incompatible vector types assign

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

[Simon Dardis via Phabricator via cfe-commits]

sdardis updated this revision to Diff 94434.
sdardis marked 7 inline comments as done.
sdardis added a comment.

Addressed review comments.

I've changed InvalidVectorOperands() to not use InvalidOperands() after 
updating some tests. InvalidOperands() was receiving expressions with implicit 
casts, leading to misleading error messages.


https://reviews.llvm.org/D25866

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaExpr.cpp
  test/Sema/vector-cast.c
  test/Sema/vector-gcc-compat.c
  test/Sema/vector-gcc-compat.cpp
  test/Sema/vector-ops.c
  test/Sema/zvector.c
  test/SemaCXX/vector-no-lax.cpp

Index: test/SemaCXX/vector-no-lax.cpp
===
--- test/SemaCXX/vector-no-lax.cpp
+++ test/SemaCXX/vector-no-lax.cpp
@@ -4,6 +4,6 @@
 
 vSInt32 foo (vUInt32 a) {
   vSInt32 b = { 0, 0, 0, 0 };
-  b += a; // expected-error{{cannot convert between vector values}}
+  b += a; // expected-error{{cannot convert between vector type 'vUInt32' (vector of 4 'unsigned int' values) and vector type 'vSInt32' (vector of 4 'int' values) as implicit conversion would cause truncation}}
   return b;
 }
Index: test/Sema/zvector.c
===
--- test/Sema/zvector.c
+++ test/Sema/zvector.c
@@ -326,14 +326,14 @@
   bc = bc + sc2; // expected-error {{incompatible type}}
   bc = sc + bc2; // expected-error {{incompatible type}}
 
-  sc = sc + sc_scalar; // expected-error {{cannot convert}}
-  sc = sc + uc_scalar; // expected-error {{cannot convert}}
-  sc = sc_scalar + sc; // expected-error {{cannot convert}}
-  sc = uc_scalar + sc; // expected-error {{cannot convert}}
-  uc = uc + sc_scalar; // expected-error {{cannot convert}}
-  uc = uc + uc_scalar; // expected-error {{cannot convert}}
-  uc = sc_scalar + uc; // expected-error {{cannot convert}}
-  uc = uc_scalar + uc; // expected-error {{cannot convert}}
+  sc = sc + sc_scalar;
+  sc = sc + uc_scalar; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  sc = sc_scalar + sc;
+  sc = uc_scalar + sc; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  uc = uc + sc_scalar; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc = uc + uc_scalar;
+  uc = sc_scalar + uc; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc = uc_scalar + uc;
 
   ss = ss + ss2;
   us = us + us2;
@@ -368,10 +368,10 @@
   sc += sl2; // expected-error {{cannot convert}}
   sc += fd2; // expected-error {{cannot convert}}
 
-  sc += sc_scalar; // expected-error {{cannot convert}}
-  sc += uc_scalar; // expected-error {{cannot convert}}
-  uc += sc_scalar; // expected-error {{cannot convert}}
-  uc += uc_scalar; // expected-error {{cannot convert}}
+  sc += sc_scalar;
+  sc += uc_scalar; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  uc += sc_scalar; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc += uc_scalar;
 
   ss += ss2;
   us += us2;
Index: test/Sema/vector-ops.c
===
--- test/Sema/vector-ops.c
+++ test/Sema/vector-ops.c
@@ -30,106 +30,108 @@
 void testLogicalVecVec(v2u v2ua, v2s v2sa, v2f v2fa) {
 
   // Logical operators
-  v2ua = v2ua && v2ua; // expected-warning {{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
-  v2ua = v2ua || v2ua; // expected-warning {{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+  v2ua = v2ua && v2ua; // expected-error {{invalid operands to binary expression ('v2u' (vector of 2 'unsigned int' values) and 'v2u')}}
+  v2ua = v2ua || v2ua; // expected-error {{invalid operands to binary expression ('v2u' (vector of 2 'unsigned int' values) and 'v2u')}}
 
-  v2ua = v2sa && v2ua; // expected-warning {{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
-  v2ua = v2sa || v2ua; // expected-warning {{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attrib

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

[Simon Dardis via Phabricator via cfe-commits]

sdardis updated this revision to Diff 95109.
sdardis marked 2 inline comments as done.
sdardis added a comment.

Addressed review comments. I've highlighted the relevant places with FIXME:s 
where we don't support GCC vector operations.


https://reviews.llvm.org/D25866

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  test/Sema/vector-cast.c
  test/Sema/vector-gcc-compat.c
  test/Sema/vector-gcc-compat.cpp
  test/Sema/vector-ops.c
  test/Sema/zvector.c
  test/SemaCXX/vector-no-lax.cpp

Index: test/SemaCXX/vector-no-lax.cpp
===
--- test/SemaCXX/vector-no-lax.cpp
+++ test/SemaCXX/vector-no-lax.cpp
@@ -4,6 +4,6 @@
 
 vSInt32 foo (vUInt32 a) {
   vSInt32 b = { 0, 0, 0, 0 };
-  b += a; // expected-error{{cannot convert between vector values}}
+  b += a; // expected-error{{cannot convert between vector type 'vUInt32' (vector of 4 'unsigned int' values) and vector type 'vSInt32' (vector of 4 'int' values) as implicit conversion would cause truncation}}
   return b;
 }
Index: test/Sema/zvector.c
===
--- test/Sema/zvector.c
+++ test/Sema/zvector.c
@@ -326,14 +326,14 @@
   bc = bc + sc2; // expected-error {{incompatible type}}
   bc = sc + bc2; // expected-error {{incompatible type}}
 
-  sc = sc + sc_scalar; // expected-error {{cannot convert}}
-  sc = sc + uc_scalar; // expected-error {{cannot convert}}
-  sc = sc_scalar + sc; // expected-error {{cannot convert}}
-  sc = uc_scalar + sc; // expected-error {{cannot convert}}
-  uc = uc + sc_scalar; // expected-error {{cannot convert}}
-  uc = uc + uc_scalar; // expected-error {{cannot convert}}
-  uc = sc_scalar + uc; // expected-error {{cannot convert}}
-  uc = uc_scalar + uc; // expected-error {{cannot convert}}
+  sc = sc + sc_scalar;
+  sc = sc + uc_scalar; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  sc = sc_scalar + sc;
+  sc = uc_scalar + sc; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  uc = uc + sc_scalar; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc = uc + uc_scalar;
+  uc = sc_scalar + uc; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc = uc_scalar + uc;
 
   ss = ss + ss2;
   us = us + us2;
@@ -368,10 +368,10 @@
   sc += sl2; // expected-error {{cannot convert}}
   sc += fd2; // expected-error {{cannot convert}}
 
-  sc += sc_scalar; // expected-error {{cannot convert}}
-  sc += uc_scalar; // expected-error {{cannot convert}}
-  uc += sc_scalar; // expected-error {{cannot convert}}
-  uc += uc_scalar; // expected-error {{cannot convert}}
+  sc += sc_scalar;
+  sc += uc_scalar; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  uc += sc_scalar; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc += uc_scalar;
 
   ss += ss2;
   us += us2;
Index: test/Sema/vector-ops.c
===
--- test/Sema/vector-ops.c
+++ test/Sema/vector-ops.c
@@ -13,11 +13,11 @@
   (void)(~v2fa); // expected-error{{invalid argument type 'v2f' (vector of 2 'float' values) to unary}}
 
   // Comparison operators
-  v2ua = (v2ua==v2sa); // expected-warning{{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+  v2ua = (v2ua==v2sa); // expected-warning{{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values}}
   v2sa = (v2ua==v2sa);
 
   // Arrays
-  int array1[v2ua]; // expected-error{{size of array has non-integer type 'v2u' (vector of 2 'unsigned int' values)}}
+  int array1[v2ua]; // expected-error{{size of array has non-integer type 'v2u' (vector of 2 'unsigned int' values}}
   int array2[17];
   // FIXME: error message below needs type!
   (void)(array2[v2ua]); // expected-error{{array subscript is not an integer}}
@@ -28,108 +28,108 @@
 }
 
 void testLogicalVecVec(v2u v2ua, v2s v2sa, v2f v2fa) {
-
   // Logical operators
-  v2ua = v2ua && v2ua; // expected-warning {{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Realised I've some comments to submit.




Comment at: lib/Sema/SemaExpr.cpp:8032
+
+  return InvalidOperands(Loc, LHS, RHS);
+}

bruno wrote:
> Double checking here: are there tests for the `InvalidOperands` case above?
Yes, this case is covered in the new diff in test/Sema/vector-ops.c and in 
test/Sema/vector-gcc-compat.c . 



Comment at: lib/Sema/SemaExpr.cpp:10024
+  // FIXME: The check for C++ here is for GCC compatibility. GCC rejects the
+  //usage of logical operators with vectors in C. This check could be
+  //notionally dropped.

bruno wrote:
> Please mention in the comment the list of logical ops that this applies to: 
> "!, &&, ||"
I've updated the relevant Check* functions with FIXME:s noting that they should 
handle vector types for compatibility with GCC.



Comment at: test/Sema/vector-gcc-compat.c:101
+
+  v2i64_r = v2i64_a && 1; // expected-error {{invalid vector operand to binary 
expression ('v2i64' (vector of 2 'long long' values))}}
+  v2i64_r = v2i64_a || 1; // expected-error {{invalid vector operand to binary 
expression ('v2i64' (vector of 2 'long long' values))}}

bruno wrote:
> Is this because of && and others only working in C++? If so, we need a better 
> error message here, along the lines of "logical expression with vector only 
> support in C++". If later on we decide to support it in non-C++, we then get 
> rid of the warning.
> 
> 
> 
Yes, these operators are oddly only available in C++.

I've changed the error message to "logical expression with vector type[s] 
'' (vector of  '' values) [and ''] 
is only supported in C++".


https://reviews.llvm.org/D25866



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29031: [mips] Add support for static model on N64

[Simon Dardis via Phabricator via cfe-commits]

sdardis created this revision.

The patch teaches the Clang driver how to handle the N64 static
relocation model properly. It enforces the correct target feature
(+noabicalls) when -fno-pic is used. This is required as non-pic
N64 code as the abi extension to call PIC code (CPIC) is unsupported.

Make PIC the default for mips64 and mips64el, this affects both N32 
& N64 ABIs, to better match GCC.

As part of this effort, clean up the assembler invocation command
builder, so the correct flags are used.


Repository:
  rL LLVM

https://reviews.llvm.org/D29031

Files:
  lib/Driver/ToolChains.cpp
  lib/Driver/Tools.cpp
  test/Driver/mips-as.c

Index: test/Driver/mips-as.c
===
--- test/Driver/mips-as.c
+++ test/Driver/mips-as.c
@@ -21,17 +21,32 @@
 // MIPS32R2-DEF-EL-AS: as{{(.exe)?}}" "-march" "mips32r2" "-mabi" "32" "-mno-shared" "-call_nonpic" "-EL"
 //
 // RUN: %clang -target mips64-linux-gnu -### \
-// RUN:   -no-integrated-as -c %s 2>&1 \
+// RUN:   -no-integrated-as -fno-pic -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=MIPS64R2-EB-AS %s
-// MIPS64R2-EB-AS: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64" "-mno-shared" "-KPIC" "-EB"
+// MIPS64R2-EB-AS: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64" "-mno-shared" "-EB"
 //
-// RUN: %clang -target mips64el-linux-gnu -### \
+// RUN: %clang -target mips64-linux-gnu -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=MIPS64R2-EB-AS-PIC %s
+// MIPS64R2-EB-AS-PIC: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64" "-EB" "-KPIC"
+//
+// RUN: %clang -target mips64el-linux-gnu -### \
+// RUN:   -no-integrated-as -c -fno-pic %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=MIPS64R2-DEF-EL-AS %s
-// MIPS64R2-DEF-EL-AS: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64"  "-mno-shared" "-KPIC" "-EL"
+// MIPS64R2-DEF-EL-AS: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64"  "-mno-shared" "-EL"
+//
+// RUN: %clang -target mips64el-linux-gnu -### \
+// RUN:   -no-integrated-as -c %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=MIPS64R2-DEF-EL-AS-PIC %s
+// MIPS64R2-DEF-EL-AS-PIC: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64" "-EL" "-KPIC"
 //
 // RUN: %clang -target mips64-linux-gnu -mabi=n32 -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=MIPS-N32-PIC %s
+// MIPS-N32-PIC: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "n32" "-call_nonpic" "-EB" "-KPIC"
+//
+// RUN: %clang -target mips64-linux-gnu -mabi=n32 -### \
+// RUN:   -no-integrated-as -c %s -fno-pic 2>&1 \
 // RUN:   | FileCheck -check-prefix=MIPS-N32 %s
 // MIPS-N32: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "n32" "-mno-shared" "-call_nonpic" "-EB"
 //
@@ -45,8 +60,13 @@
 //
 // RUN: %clang -target mips64el-linux-gnu -mabi=64 -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=MIPS64R2-EL-AS-PIC %s
+// MIPS64R2-EL-AS-PIC: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64" "-EL" "-KPIC"
+//
+// RUN: %clang -target mips64el-linux-gnu -mabi=64 -### \
+// RUN:   -no-integrated-as -c %s -fno-pic 2>&1 \
 // RUN:   | FileCheck -check-prefix=MIPS64R2-EL-AS %s
-// MIPS64R2-EL-AS: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64" "-mno-shared" "-KPIC" "-EL"
+// MIPS64R2-EL-AS: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64" "-mno-shared" "-EL"
 //
 // RUN: %clang -target mips-linux-gnu -march=mips32r2 -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
@@ -60,8 +80,13 @@
 //
 // RUN: %clang -target mips64-linux-gnu -march=octeon -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=MIPS-OCTEON-PIC %s
+// MIPS-OCTEON-PIC: as{{(.exe)?}}" "-march" "octeon" "-mabi" "64" "-EB" "-KPIC"
+//
+// RUN: %clang -target mips64-linux-gnu -march=octeon -### \
+// RUN:   -no-integrated-as -c %s -fno-pic 2>&1 \
 // RUN:   | FileCheck -check-prefix=MIPS-OCTEON %s
-// MIPS-OCTEON: as{{(.exe)?}}" "-march" "octeon" "-mabi" "64" "-mno-shared" "-KPIC" "-EB"
+// MIPS-OCTEON: as{{(.exe)?}}" "-march" "octeon" "-mabi" "64" "-mno-shared" "-EB"
 //
 // RUN: %clang -target mips-linux-gnu -mips1 -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
@@ -115,28 +140,48 @@
 //
 // RUN: %clang -target mips64-linux-gnu -mips64 -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=MIPS-ALIAS-64-PIC %s
+// MIPS-ALIAS-64-PIC: as{{(.exe)?}}" "-march" "mips64" "-mabi" "64" "-EB" "-KPIC"
+//
+// RUN: %clang -target mips64-linux-gnu -mips64 -### \
+// RUN:   -no-integrated-as -c -fno-pic %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=MIPS-ALIAS-64 %s
-// MIPS-ALIAS-64: as{{(.exe)?}}" "-march" "mips64" "-mabi" "64" "-mno-shared" "-KPIC" "-EB"
+// MIPS-ALIAS-64: as{{(.exe)?}}" "-march" "mips64" "-mabi" "64" "-mno-shared" "-EB"
 //
 // RUN: %clang -target mips64-linux-gnu -mips64r2 -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=MIPS-ALIAS-64R2 %s
-// MIPS-ALIAS-64R2: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64" "-mno

[PATCH] D29032: [mips] Define macros related to -mabicalls in the preprocessor

[Simon Dardis via Phabricator via cfe-commits]

sdardis created this revision.

Historically, NetBSD, FreeBSD and OpenBSD have defined the macro ABICALLS in
the preprocessor when -mabicalls is in effect.

Mainline GCC later defined __mips_abicalls when -mabicalls is in effect.

This patch teaches the preprocessor to define these macros when appropriate.

This resolves PR/31694.

Thanks to Sean Bruno for highlighting this issue!


https://reviews.llvm.org/D29032

Files:
  lib/Basic/Targets.cpp
  test/Preprocessor/init.c

Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -3040,6 +3040,7 @@
 // MIPS32BE:#define __llvm__ 1
 // MIPS32BE:#define __mips 32
 // MIPS32BE:#define __mips__ 1
+// MIPS32BE:#define __mips_abicalls 1
 // MIPS32BE:#define __mips_fpr 32
 // MIPS32BE:#define __mips_hard_float 1
 // MIPS32BE:#define __mips_o32 1
@@ -3246,6 +3247,7 @@
 // MIPS32EL:#define __llvm__ 1
 // MIPS32EL:#define __mips 32
 // MIPS32EL:#define __mips__ 1
+// MIPS32EL:#define __mips_abicalls 1
 // MIPS32EL:#define __mips_fpr 32
 // MIPS32EL:#define __mips_hard_float 1
 // MIPS32EL:#define __mips_o32 1
@@ -3555,6 +3557,7 @@
 // MIPSN32BE: #define __mips64 1
 // MIPSN32BE: #define __mips64__ 1
 // MIPSN32BE: #define __mips__ 1
+// MIPSN32BE: #define __mips_abicalls 1
 // MIPSN32BE: #define __mips_fpr 64
 // MIPSN32BE: #define __mips_hard_float 1
 // MIPSN32BE: #define __mips_isa_rev 2
@@ -3861,6 +3864,7 @@
 // MIPSN32EL: #define __mips64 1
 // MIPSN32EL: #define __mips64__ 1
 // MIPSN32EL: #define __mips__ 1
+// MIPSN32EL: #define __mips_abicalls 1
 // MIPSN32EL: #define __mips_fpr 64
 // MIPSN32EL: #define __mips_hard_float 1
 // MIPSN32EL: #define __mips_isa_rev 2
@@ -4073,6 +4077,7 @@
 // MIPS64BE:#define __mips64 1
 // MIPS64BE:#define __mips64__ 1
 // MIPS64BE:#define __mips__ 1
+// MIPS64BE:#define __mips_abicalls 1
 // MIPS64BE:#define __mips_fpr 64
 // MIPS64BE:#define __mips_hard_float 1
 // MIPS64BE:#define __mips_n64 1
@@ -4282,6 +4287,7 @@
 // MIPS64EL:#define __mips64 1
 // MIPS64EL:#define __mips64__ 1
 // MIPS64EL:#define __mips__ 1
+// MIPS64EL:#define __mips_abicalls 1
 // MIPS64EL:#define __mips_fpr 64
 // MIPS64EL:#define __mips_hard_float 1
 // MIPS64EL:#define __mips_n64 1
@@ -4513,6 +4519,45 @@
 // MIPS-XXR6:#define __mips_fpr 64
 // MIPS-XXR6:#define __mips_nan2008 1
 //
+// RUN: %clang_cc1 -target-cpu mips32 \
+// RUN:   -E -dM -triple=mips-unknown-netbsd -mrelocation-model pic < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefix MIPS-ABICALLS-NETBSD %s
+// MIPS-ABICALLS-NETBSD: #define __ABICALLS__ 1
+// MIPS-ABICALLS-NETBSD-NOT: #define __mips_abicalls 1
+//
+// RUN: %clang_cc1 -target-cpu mips64 \
+// RUN:   -E -dM -triple=mips64-unknown-netbsd -mrelocation-model pic < \
+// RUN:   /dev/null | FileCheck -match-full-lines \
+// RUN:   -check-prefix MIPS-ABICALLS-NETBSD64 %s
+// MIPS-ABICALLS-NETBSD64: #define __ABICALLS__ 1
+// MIPS-ABICALLS-NETBSD-NOT: #define __mips_abicalls 1
+//
+// RUN: %clang_cc1 -target-cpu mips32 \
+// RUN:   -E -dM -triple=mips-unknown-freebsd -mrelocation-model pic < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefix MIPS-ABICALLS-FREEBSD %s
+// MIPS-ABICALLS-FREEBSD: #define __ABICALLS__ 1
+// MIPS-ABICALLS-FREEBSD-NOT: #define __mips_abicalls 1
+//
+// RUN: %clang_cc1 -target-cpu mips64 \
+// RUN:   -E -dM -triple=mips64-unknown-freebsd -mrelocation-model pic < \
+// RUN:   /dev/null | FileCheck -match-full-lines \
+// RUN:   -check-prefix MIPS-ABICALLS-FREEBSD64 %s
+// MIPS-ABICALLS-FREEBSD64: #define __ABICALLS__ 1
+// MIPS-ABICALLS-FREEBSD-NOT: #define __mips_abicalls 1
+//
+// RUN: %clang_cc1 -target-cpu mips32 \
+// RUN:   -E -dM -triple=mips-unknown-openbsd -mrelocation-model pic < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefix MIPS-ABICALLS-OPENBSD %s
+// MIPS-ABICALLS-OPENBSD: #define __ABICALLS__ 1
+// MIPS-ABICALLS-OPENBSD-NOT: #define __mips_abicalls 1
+//
+// RUN: %clang_cc1 -target-cpu mips64 \
+// RUN:   -E -dM -triple=mips64-unknown-openbsd -mrelocation-model pic < \
+// RUN:   /dev/null | FileCheck -match-full-lines \
+// RUN:   -check-prefix MIPS-ABICALLS-OPENBSD64 %s
+// MIPS-ABICALLS-OPENBSD64: #define __ABICALLS__ 1
+// MIPS-ABICALLS-OPENBSD-NOT: #define __mips_abicalls 1
+//
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=msp430-none-none < /dev/null | FileCheck -match-full-lines -check-prefix MSP430 %s
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=msp430-none-none < /dev/null | FileCheck -match-full-lines -check-prefix MSP430 -check-prefix MSP430-CXX %s
 //
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -7434,6 +7434,8 @@
   bool IsMicromips;
   bool IsNan2008;
   bool IsSingleFloat;
+  bool IsNoABICalls;
+  bool CanUseBSDABICalls;
   enum MipsFloatABI {
 HardFloat, SoftFloat
   } FloatABI;
@@ -7449,16 +7451,21 @@

[PATCH] D29032: [mips] Define macros related to -mabicalls in the preprocessor

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Sending it to the correct list this time.


https://reviews.llvm.org/D29032



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits