[PATCH] D50477: WIP: Ensure that the type of size_t is represended as one of the fixed width types

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I don't understand the desire for this logic. Why can't wasm override the rest 
of the types if it wants to have something special?


Repository:
  rC Clang

https://reviews.llvm.org/D50477



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

[PATCH] D50839: [llvm] Optimize YAML::isNumeric

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added inline comments.



Comment at: llvm/include/llvm/Support/YAMLTraits.h:477
+  if (ParseOct && std::find(std::begin(OctalChars), std::end(OctalChars),
+Char) == std::end(OctalChars))
+return false;

Can you use strchr here? I would expect the compiler to fold the string 
constants into bit tests, creating both more compact and faster code.


https://reviews.llvm.org/D50839



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

[PATCH] D50839: [llvm] Optimize YAML::isNumeric

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added inline comments.



Comment at: 
llvm/tools/llvm-yaml-numeric-parser-fuzzer/yaml-numeric-parser-fuzzer.cpp:16
+
+llvm::Regex Inifnity("^[-+]?(\\.inf|\\.Inf|\\.INF)$");
+llvm::Regex Base8("^0o[0-7]+$");

Spelling?


https://reviews.llvm.org/D50839



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

[PATCH] D50966: Fix an undefined behavior when storing an empty StringRef.

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Why do we need to allocate memory in this case at all? I.e. why can't this just 
be:

  if (S.empty())
return StringRef("", 0);
  ...


Repository:
  rL LLVM

https://reviews.llvm.org/D50966



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

[PATCH] D50413: [libunwind][include] Add some missing definitions to .

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Is there a reason for defining them? As in: does anything outside libunwind use 
them? I haven't seen such software yet.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50413



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

[PATCH] D47138: [Sparc] Use the leon arch for Leon3's when using an external assembler

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

This still needs a test case?


https://reviews.llvm.org/D47138



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

[PATCH] D38861: [CodeGen] Error on unsupported checked multiplies early

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:2263
+  }
+}
+

vsk wrote:
> rjmccall wrote:
> > Is there a reason this only fails on x86?  If LLVM doesn't have generic 
> > wide-operation lowering, this probably needs to be a target whitelist 
> > rather than a blacklist.
> That makes sense. For the 128-bit operation, the whitelist is {x86-64, 
> wasm64, mips64}. We don't support this operation for bit widths larger than 
> 128 bits on any target. I'll update the patch accordingly.
That sounds wrong. __int128_t should be supported by all 64bit architectures, 
not just those three.


https://reviews.llvm.org/D38861



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

[PATCH] D37954: Try to shorten system header paths when using -MD depfiles

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

The behavior of sometimes transforming the path and sometimes not is IMO 
completely unacceptable. I don't care if GCC created that bug first.


Repository:
  rL LLVM

https://reviews.llvm.org/D37954



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

[PATCH] D37954: Try to shorten system header paths when using -MD depfiles

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

This is not about any operating system, but basic consistent behavior. either 
do the canonicalisation or not. Doing it sometimes is just bogus. You've 
effectively implemented -fcanonical-system-headers=sometimes.


Repository:
  rL LLVM

https://reviews.llvm.org/D37954



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

[PATCH] D39162: [test] Fix clang-test for FreeBSD and NetBSD

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I think we should special case Darwin and Windows and fall-back to 
LD_LIBRARY_PATH for the rest. Can't remember if there is a UNIX-like platform 
left where it doesn't work.


https://reviews.llvm.org/D39162



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

[PATCH] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I've looked at this in some detail now. I'm not exactly sure yet why it is 
broken. The patch seems quite wrong to me. DW_CFA_GNU_args_size should be 
applied only when unwinding a call instruction and that regard, the commit 
message of the original change is quite correct. What I am still trying to 
understand is how the precise unwind frame disagrees with the unwinder.


https://reviews.llvm.org/D38680



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Why again is this a good idea? This is an even worse hack than -Bsymbolic, the 
latter at least is visible in ELF header without code inspection. This is 
breaking core premises of ELF.


https://reviews.llvm.org/D39079



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Let me phrase it differently. What is this patch (and the matching backend PR) 
supposed to achieve? There are effectively two ways to get rid of PLT entries:
(1) Bind references locally. This is effectively what -Bsymbolic does and what 
is breaking the ELF interposition rules.
(2) Do an indirect call via the GOT. Requires knowing what  an external symbol 
is, making it non-attractive for anything but LTO, since it will create 
performance issues for all non-local accesses (i.e. anything private).


https://reviews.llvm.org/D39079



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

In https://reviews.llvm.org/D39079#905396, @rnk wrote:

> In https://reviews.llvm.org/D39079#905372, @joerg wrote:
>
> > Why again is this a good idea?
>
>
> It saves the direct jump to the PLT, reducing icache pressure, which is a 
> major cost in some workloads.


It also increases the pressure on the branch predictor, so it is not really 
black and white.

>> This is an even worse hack than -Bsymbolic,
> 
> Personally, I would like to build LLVM with -Bsymbolic so that we can build 
> LLVM as a DSO and load it from clang without regressing startup time, so I 
> don't see what's so terrible about -Bsymbolic, especially for C++ programs.

Qt5 tries that. Requires further hacks as the main binary must be compiled as 
fully position independent code to not run into fun latter. Fun with copy 
relocations is only part of it.

> Anyway, LLVM already has an attribute, nonlazybind, and this just provides a 
> flag to apply it to all declarations. It gives the user access to the 
> GOTPCREL relocations that we, and loaders, already support.

The loader doesn't see GOTPCREL anymore. It also requires a linker that 
disassembles instructions, because it can't distinguish between a normal 
pointer load and a call, to be able to optimize it.


https://reviews.llvm.org/D39079



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

In https://reviews.llvm.org/D39079#905468, @rnk wrote:

> In https://reviews.llvm.org/D39079#905454, @joerg wrote:
>
> > It also increases the pressure on the branch predictor, so it is not really 
> > black and white.
>
>
> I don't understand this objection. I'm assuming that the PLT stub is an 
> indirect jump through the PLTGOT,
>  not a hotpatched stub that jumps directly to the definition chosen by the 
> loader. This is the ELF model
>  that I'm familiar with, especially since calls to code more than 2GB away 
> generally need to be indirect anyway.


Correct, so all local calls to the same function go via the same location and 
share the predication of the indirect jump.

>> Qt5 tries that. Requires further hacks as the main binary must be compiled 
>> as fully position independent code to not run into fun latter. Fun with copy 
>> relocations is only part of it.
> 
> I'm not sure I understand, but this patch isn't introducing copy relocations, 
> to be clear.

That was in reference to using it for clang.


https://reviews.llvm.org/D39079



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

[PATCH] D43871: [modules] No longer include stdlib.h from mm_malloc.h.

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Please check the history of the file for some of the problems with the 
redefinition. I'm quite against this change.


https://reviews.llvm.org/D43871



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

[PATCH] D51683: Fix arm_neon.h and arm_fp16.h generation for compiling with std=c89

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added inline comments.



Comment at: cfe/trunk/utils/TableGen/NeonEmitter.cpp:2412
 
-  OS << "#define __ai static inline __attribute__((__always_inline__, "
+  OS << "#define __ai static __inline __attribute__((__always_inline__, "
 "__nodebug__))\n\n";

If you want to change it, at least change it properly to use __inline__.



Comment at: cfe/trunk/utils/TableGen/NeonEmitter.cpp:2521
 
-  OS << "#define __ai static inline __attribute__((__always_inline__, "
+  OS << "#define __ai static __inline __attribute__((__always_inline__, "
 "__nodebug__))\n\n";

Same.


Repository:
  rL LLVM

https://reviews.llvm.org/D51683



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

[PATCH] D51683: Fix arm_neon.h and arm_fp16.h generation for compiling with std=c89

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Correct. The protected name is double underscore as both suffix and prefix.


https://reviews.llvm.org/D51683



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

[PATCH] D49763: [CUDA] Call atexit() for CUDA destructor early on.

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Can this ever end up in a shared library? If yes, please use the normal logic 
for creating a global destructor. atexit is not very friendly to dlopen...


https://reviews.llvm.org/D49763



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

[PATCH] D49763: [CUDA] Call atexit() for CUDA destructor early on.

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Depends a bit on the platform, __cxa_atexit on most modern ELF systems, 
fallback to atexit. If the global dtor is run too late, it smells like a 
missing library dependency. They are executed in topological order after all.


https://reviews.llvm.org/D49763



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

[PATCH] D49771: CodeGen: use non-zero memset when possible for automatic variables

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

There are two different considerations here:
(1) Create less target code
(2) Create less IR

If this code can significantly reduce the amount of IR, it can be useful in 
general. That's why the existing memset logic is helpful.


Repository:
  rL LLVM

https://reviews.llvm.org/D49771



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

[PATCH] D42593: GCC compatibility: Ignore -fstack-clash-protection

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I really don't like ignoring options that are supposed to provide actual 
functionality. Most of the other options are for pointless fine tuning and 
workarounds for broken gcc behavior in ancient versions.


Repository:
  rC Clang

https://reviews.llvm.org/D42593



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

[PATCH] D33726: [driver][netbsd] Build and pass `-L` arguments to the linker

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I'm against it. I consider this a strong bug on the LLD side and the behavior 
of clang correct.


Repository:
  rL LLVM

https://reviews.llvm.org/D33726



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

[PATCH] D33726: [driver][netbsd] Build and pass `-L` arguments to the linker

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Such knowledge is necessary anyway. There is enough software that wants to use 
the  linker directly.


Repository:
  rL LLVM

https://reviews.llvm.org/D33726



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

[PATCH] D33726: [driver][netbsd] Build and pass `-L` arguments to the linker

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

sysroot is already handled in NetBSD.cpp line 118 or so.


Repository:
  rL LLVM

https://reviews.llvm.org/D33726



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

[PATCH] D33726: [driver][netbsd] Build and pass `-L` arguments to the linker

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

A small subset can be found by searching for LINKER_RPATH_FLAG in pkgsrc. A 
classic offender is Emacs. For more, I would have to systematically search.


Repository:
  rL LLVM

https://reviews.llvm.org/D33726



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

[PATCH] D31972: Do not force the frame pointer by default for ARM EABI

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Actually, for NetBSD we want to use -fomit-frame-pointer by default whenever 
the implicit -funwind-tables is also present. In general, that should be the 
justification for the default behavior: "Can I reliably unwind a binary with 
the available information?" Performance of stack unwinding is IMO secondary and 
users of sanitizers can disable it where necessary.


https://reviews.llvm.org/D31972



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

[PATCH] D33726: [driver][netbsd] Build and pass `-L` arguments to the linker

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

In https://reviews.llvm.org/D33726#774105, @ruiu wrote:

> I'm totally against adding per-OS path knowledge to our linker. Compilers 
> already know include paths and I don't want to maintain another list of paths 
> in the linker. Also this can be more confusing than useful when you are doing 
> cross-linking.


The only reason for compilers to maintain that list is for finding crt*.o. They 
otherwise don't care about the library paths at all. There is no confusion for 
cross-linking as long as proper sysroot support is used. Which we have been 
doing on NetBSD for ages.

> For all OSes other than NetBSD, LLD works fine with the clang driver as the 
> driver passes include paths to the linker. I don't see any reason not to do 
> the same thing for NetBSD. That stands even if the linker has to have a list 
> of include paths.

Sorry, but this is again ignorant and wrong. The very same problem of build 
systems calling ld directly apply on most other systems. Even then, the list of 
linker paths is not the only OS-specific knowledge. Things like the DT_RPATH vs 
DT_RUNPATH mess, init vs init_array all belong into this category. The list 
goes on.


Repository:
  rL LLVM

https://reviews.llvm.org/D33726



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

[PATCH] D34018: Support __float128 on NetBSD libstdc++ x86/x86_64

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

At the very least, missing test case.

I'm mostly ambivalent about this -- I don't really see the point and without 
matching soft float support it won't fully work either.


Repository:
  rL LLVM

https://reviews.llvm.org/D34018



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

[PATCH] D34018: Support __float128 on NetBSD libstdc++ x86/x86_64

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Soft-float on the runtime environment part. I.e. non-trivial operations are 
lowered to library calls.


Repository:
  rL LLVM

https://reviews.llvm.org/D34018



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

[PATCH] D34018: Support __float128 on NetBSD libstdc++ x86/x86_64

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

As I said, I don't see the point in pretending we support float128 when the 
runtime doesn't contain the necessary pieces.


Repository:
  rL LLVM

https://reviews.llvm.org/D34018



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

[PATCH] D34506: Relax an assert in the comparison of source locations

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I don't think it is a good idea to make this function non-transitive.


Repository:
  rL LLVM

https://reviews.llvm.org/D34506



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

[PATCH] D41054: Teach clang/NetBSD about additional dependencies for sanitizers

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I'm not really a fan of linking libutil into all binaries. Why is this code 
using forkpty in first place and not posix_openpt/grantpt?


Repository:
  rL LLVM

https://reviews.llvm.org/D41054



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

[PATCH] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg updated this revision to Diff 150049.
joerg added a comment.

After a careful review of newer GCC / libgcc and the assembler annotations from 
LLVM, I have come to the following conclusions:

(1) The semantics have been somewhat changed by GCC in recent years. There is 
no actual specification, so we have to go by what behavior actually makes sense.
(2) The primary motivation is still that the DW_CFA_GNU_args_size is a 
call-site specific annotation. It is expected to be applied when the IP is 
moved by the personality routine to compensate for the call site specific 
(temporary) adjustment.
(3) It is not clear with plain unw_set_ip outside the scope of the Itanium EH 
handling should have this behavior, so it might need to be split into an 
internal routine.
(4) LLVM does not produce correct CFA annotation for stdcall and similar cases 
where the callee removes additional stack space.

The patch covers the first two items.


https://reviews.llvm.org/D38680

Files:
  src/UnwindCursor.hpp
  src/libunwind.cpp


Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -188,8 +188,13 @@
 co->setReg(regNum, (pint_t)value);
 // specical case altering IP to re-find info (being called by personality
 // function)
-if (regNum == UNW_REG_IP)
+if (regNum == UNW_REG_IP) {
+  unw_proc_info_t info;
+  co->getInfo(&info);
   co->setInfoBasedOnIPRegister(false);
+  if (info.gp)
+co->setReg(UNW_REG_SP, co->getReg(UNW_REG_SP) + info.gp);
+}
 return UNW_ESUCCESS;
   }
   return UNW_EBADREG;
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -1411,8 +1411,6 @@
 this->setInfoBasedOnIPRegister(true);
 if (_unwindInfoMissing)
   return UNW_STEP_END;
-if (_info.gp)
-  setReg(UNW_REG_SP, getReg(UNW_REG_SP) + _info.gp);
   }
 
   return result;


Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -188,8 +188,13 @@
 co->setReg(regNum, (pint_t)value);
 // specical case altering IP to re-find info (being called by personality
 // function)
-if (regNum == UNW_REG_IP)
+if (regNum == UNW_REG_IP) {
+  unw_proc_info_t info;
+  co->getInfo(&info);
   co->setInfoBasedOnIPRegister(false);
+  if (info.gp)
+co->setReg(UNW_REG_SP, co->getReg(UNW_REG_SP) + info.gp);
+}
 return UNW_ESUCCESS;
   }
   return UNW_EBADREG;
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -1411,8 +1411,6 @@
 this->setInfoBasedOnIPRegister(true);
 if (_unwindInfoMissing)
   return UNW_STEP_END;
-if (_info.gp)
-  setReg(UNW_REG_SP, getReg(UNW_REG_SP) + _info.gp);
   }
 
   return result;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34796: upporting -f(no)-reorder-functions flag, clang side change

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Excuse me for bring this up so late, but why do we want to make any such 
promises? As in: fundamentally, LLVM IR doesn't have any order property on the 
module level. I have yet so seen reasonable code where the order of functions 
matters for anything but performance. I've seen a few things that required 
`-funit-at-a-time`, most noticable GCC's CRT implementation. But those are all 
major hacks. So under what premise is it useful to have to even pretend to 
honor source code order?


Repository:
  rC Clang

https://reviews.llvm.org/D34796



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added inline comments.



Comment at: ELF/Driver.cpp:779
+// TODO: verify the triple somehow?
+Config->TargetTriple = llvm::Triple(Prefix);
+  }

mgorny wrote:
> joerg wrote:
> > See ToolChain::getTargetAndModeFromProgramName in clang.
> Yes, I've based this on stripped down version of that. Most notably, I wasn't 
> able to verify triple against TargetRegistry since it isn't initialized here.
initLLVM is called too late. That should be all that is needed to do a proper 
look up.


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

https://reviews.llvm.org/D56215



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

[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

That's the other reason why I find the GCC specification as string prefix 
confusing. I still say we should just go with mapping of path names and then 
the order question mostly goes away.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Right, I'm not aware of anyone but FreeBSD really using the OSABI field. For 
FreeBSD it was a long standing hack around limitations in the ELF kernel 
loader. I'm not even sure if FreeBSD use(d) to set the OSABI field for the 
intermediate object files.


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

https://reviews.llvm.org/D56215



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

[PATCH] D56647: [WIP] [ELF] Implement --copy-dt-needed-entries

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

As first step, it goes into the right direction. I would explicitly set 
--as-needed for all those indirectly loaded objects. If people want to retain 
the questionable behavior of newer GNU tools, it could be a separate flag so 
that a final round can warn if an indirectly pulled library is necessary, but 
that behavior doesn't IMO make much sense. Full version has to look at 
DT_RUNPATH/DT_RPATH and also --rpath-link.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56647



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

[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

As discussed with dankm on IRC, I still would like to see the correct behavior 
going into 8.0, i.e. not change it later. Since this also matters for potential 
faster implementations later, it seems like a good idea to do it now. The 
changes are well-localized.

(1) Do path prefix matching and not string prefix matching. The difference is 
that the file name must be longer than the prefix and the prefix must be 
followed by a path separator.
(2) The longest prefix match wins. Substituation is applied only once per file 
name, independent of the rules. This gives more predictable output and allows 
switching to a tree-lookup later.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

[PATCH] D34018: Support __float128 on NetBSD libstdc++ x86/x86_64

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Nothing changed. I don't see how catering to the broken libstdc++ 
self-configuration helps. So no, I still object to this change.


Repository:
  rL LLVM

https://reviews.llvm.org/D34018



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

[PATCH] D54657: [clang] Add -MJJ for appending to compilation databases.

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I don't understand the point here. Why would you want to include 
pre-processing-only commands in the compilation database?


Repository:
  rC Clang

https://reviews.llvm.org/D54657



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

[PATCH] D54657: [clang] Add -MJJ for appending to compilation databases.

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I'm sorry, but it still sounds to me like you want to address badly written 
build rules by making the driver more complicated. I don't see that is a 
reasonable goal forward.


Repository:
  rC Clang

https://reviews.llvm.org/D54657



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

[PATCH] D55121: Make several Python scripts portable across Python2 and Python 3

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Can you split off the pure modernisation changes like new exception or print ? 
Those are completely non-contentious changes after all. I generally do not like 
the range and list related changes as many instances are clear regressions for 
the 2.x case. filter to list comprehension should IMO be a separate change as 
well, but those are much less problematic and often an improvement in terms of 
both performance and readability.


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

https://reviews.llvm.org/D55121



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

[PATCH] D43871: [modules] No longer include stdlib.h from mm_malloc.h.

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

This header is used on systems without glibc. So please don't argue about 
behavior based only on that. Granted, most other libc implementation are less 
annoying when it comes to `free` and `malloc`, but still.


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

https://reviews.llvm.org/D43871



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

[PATCH] D58649: Fix inline assembler constraint validation

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg created this revision.
joerg added a reviewer: compnerd.
Herald added a subscriber: eraman.

The current constraint logic is both too lax and too strict. It fails for input 
outside the [INT_MIN..INT_MAX] range, but it also implicitly accepts 0 as value 
when it should not. Adjust logic to handle both correctly.


https://reviews.llvm.org/D58649

Files:
  include/clang/Basic/TargetInfo.h
  test/Sema/inline-asm-validate-x86.c


Index: test/Sema/inline-asm-validate-x86.c
===
--- test/Sema/inline-asm-validate-x86.c
+++ test/Sema/inline-asm-validate-x86.c
@@ -55,6 +55,7 @@
 void L(int i, int j) {
   static const int Invalid1 = 1;
   static const int Invalid2 = 42;
+  static const int Invalid3 = 0;
   static const int Valid1 = 0xff;
   static const int Valid2 = 0x;
   static const int Valid3 = 0x;
@@ -69,6 +70,9 @@
   : "0"(i), "L"(Invalid2)); // expected-error{{value '42' out of range 
for constraint 'L'}}
   __asm__("xorl %0,%2"
   : "=r"(i)
+  : "0"(i), "L"(Invalid3)); // expected-error{{value '0' out of range 
for constraint 'L'}}
+  __asm__("xorl %0,%2"
+  : "=r"(i)
   : "0"(i), "L"(Valid1)); // expected-no-error
   __asm__("xorl %0,%2"
   : "=r"(i)
Index: include/clang/Basic/TargetInfo.h
===
--- include/clang/Basic/TargetInfo.h
+++ include/clang/Basic/TargetInfo.h
@@ -816,6 +816,7 @@
 struct {
   int Min;
   int Max;
+  bool isConstrained;
 } ImmRange;
 llvm::SmallSet ImmSet;
 
@@ -826,6 +827,7 @@
 : Flags(0), TiedOperand(-1), ConstraintStr(ConstraintStr.str()),
   Name(Name.str()) {
   ImmRange.Min = ImmRange.Max = 0;
+  ImmRange.isConstrained = false;
 }
 
 const std::string &getConstraintStr() const { return ConstraintStr; }
@@ -854,8 +856,9 @@
   return (Flags & CI_ImmediateConstant) != 0;
 }
 bool isValidAsmImmediate(const llvm::APInt &Value) const {
-  return (Value.sge(ImmRange.Min) && Value.sle(ImmRange.Max)) ||
- ImmSet.count(Value.getZExtValue()) != 0;
+  if (!ImmSet.empty())
+return ImmSet.count(Value.getZExtValue()) != 0;
+  return !ImmRange.isConstrained || (Value.sge(ImmRange.Min) && 
Value.sle(ImmRange.Max));
 }
 
 void setIsReadWrite() { Flags |= CI_ReadWrite; }
@@ -867,6 +870,7 @@
   Flags |= CI_ImmediateConstant;
   ImmRange.Min = Min;
   ImmRange.Max = Max;
+  ImmRange.isConstrained = true;
 }
 void setRequiresImmediate(llvm::ArrayRef Exacts) {
   Flags |= CI_ImmediateConstant;
@@ -879,8 +883,6 @@
 }
 void setRequiresImmediate() {
   Flags |= CI_ImmediateConstant;
-  ImmRange.Min = INT_MIN;
-  ImmRange.Max = INT_MAX;
 }
 
 /// Indicate that this is an input operand that is tied to


Index: test/Sema/inline-asm-validate-x86.c
===
--- test/Sema/inline-asm-validate-x86.c
+++ test/Sema/inline-asm-validate-x86.c
@@ -55,6 +55,7 @@
 void L(int i, int j) {
   static const int Invalid1 = 1;
   static const int Invalid2 = 42;
+  static const int Invalid3 = 0;
   static const int Valid1 = 0xff;
   static const int Valid2 = 0x;
   static const int Valid3 = 0x;
@@ -69,6 +70,9 @@
   : "0"(i), "L"(Invalid2)); // expected-error{{value '42' out of range for constraint 'L'}}
   __asm__("xorl %0,%2"
   : "=r"(i)
+  : "0"(i), "L"(Invalid3)); // expected-error{{value '0' out of range for constraint 'L'}}
+  __asm__("xorl %0,%2"
+  : "=r"(i)
   : "0"(i), "L"(Valid1)); // expected-no-error
   __asm__("xorl %0,%2"
   : "=r"(i)
Index: include/clang/Basic/TargetInfo.h
===
--- include/clang/Basic/TargetInfo.h
+++ include/clang/Basic/TargetInfo.h
@@ -816,6 +816,7 @@
 struct {
   int Min;
   int Max;
+  bool isConstrained;
 } ImmRange;
 llvm::SmallSet ImmSet;
 
@@ -826,6 +827,7 @@
 : Flags(0), TiedOperand(-1), ConstraintStr(ConstraintStr.str()),
   Name(Name.str()) {
   ImmRange.Min = ImmRange.Max = 0;
+  ImmRange.isConstrained = false;
 }
 
 const std::string &getConstraintStr() const { return ConstraintStr; }
@@ -854,8 +856,9 @@
   return (Flags & CI_ImmediateConstant) != 0;
 }
 bool isValidAsmImmediate(const llvm::APInt &Value) const {
-  return (Value.sge(ImmRange.Min) && Value.sle(ImmRange.Max)) ||
- ImmSet.count(Value.getZExtValue()) != 0;
+  if (!ImmSet.empty())
+return ImmSet.count(Value.getZExtValue()) != 0;
+  return !ImmRange.isConstrained || (Value.sge(ImmRange.Min) && Value.sle(ImmRange.Max));
 }
 
 void setIsReadWrite() { Flags |= CI_ReadWrite; }
@@ -867,6 +870,7 @@
   Flags |= CI_ImmediateConstant;
   ImmRange.Min = Min;
   ImmRange.Max = Max;
+  

[PATCH] D58649: Fix inline assembler constraint validation

[Joerg Sonnenberger via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354937: Fix inline assembler constraint validation (authored 
by joerg, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58649?vs=188265&id=188477#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58649

Files:
  cfe/trunk/include/clang/Basic/TargetInfo.h
  cfe/trunk/test/Sema/inline-asm-validate-x86.c


Index: cfe/trunk/include/clang/Basic/TargetInfo.h
===
--- cfe/trunk/include/clang/Basic/TargetInfo.h
+++ cfe/trunk/include/clang/Basic/TargetInfo.h
@@ -816,6 +816,7 @@
 struct {
   int Min;
   int Max;
+  bool isConstrained;
 } ImmRange;
 llvm::SmallSet ImmSet;
 
@@ -826,6 +827,7 @@
 : Flags(0), TiedOperand(-1), ConstraintStr(ConstraintStr.str()),
   Name(Name.str()) {
   ImmRange.Min = ImmRange.Max = 0;
+  ImmRange.isConstrained = false;
 }
 
 const std::string &getConstraintStr() const { return ConstraintStr; }
@@ -854,8 +856,9 @@
   return (Flags & CI_ImmediateConstant) != 0;
 }
 bool isValidAsmImmediate(const llvm::APInt &Value) const {
-  return (Value.sge(ImmRange.Min) && Value.sle(ImmRange.Max)) ||
- ImmSet.count(Value.getZExtValue()) != 0;
+  if (!ImmSet.empty())
+return ImmSet.count(Value.getZExtValue()) != 0;
+  return !ImmRange.isConstrained || (Value.sge(ImmRange.Min) && 
Value.sle(ImmRange.Max));
 }
 
 void setIsReadWrite() { Flags |= CI_ReadWrite; }
@@ -867,6 +870,7 @@
   Flags |= CI_ImmediateConstant;
   ImmRange.Min = Min;
   ImmRange.Max = Max;
+  ImmRange.isConstrained = true;
 }
 void setRequiresImmediate(llvm::ArrayRef Exacts) {
   Flags |= CI_ImmediateConstant;
@@ -879,8 +883,6 @@
 }
 void setRequiresImmediate() {
   Flags |= CI_ImmediateConstant;
-  ImmRange.Min = INT_MIN;
-  ImmRange.Max = INT_MAX;
 }
 
 /// Indicate that this is an input operand that is tied to
Index: cfe/trunk/test/Sema/inline-asm-validate-x86.c
===
--- cfe/trunk/test/Sema/inline-asm-validate-x86.c
+++ cfe/trunk/test/Sema/inline-asm-validate-x86.c
@@ -55,6 +55,7 @@
 void L(int i, int j) {
   static const int Invalid1 = 1;
   static const int Invalid2 = 42;
+  static const int Invalid3 = 0;
   static const int Valid1 = 0xff;
   static const int Valid2 = 0x;
   static const int Valid3 = 0x;
@@ -69,6 +70,9 @@
   : "0"(i), "L"(Invalid2)); // expected-error{{value '42' out of range 
for constraint 'L'}}
   __asm__("xorl %0,%2"
   : "=r"(i)
+  : "0"(i), "L"(Invalid3)); // expected-error{{value '0' out of range 
for constraint 'L'}}
+  __asm__("xorl %0,%2"
+  : "=r"(i)
   : "0"(i), "L"(Valid1)); // expected-no-error
   __asm__("xorl %0,%2"
   : "=r"(i)


Index: cfe/trunk/include/clang/Basic/TargetInfo.h
===
--- cfe/trunk/include/clang/Basic/TargetInfo.h
+++ cfe/trunk/include/clang/Basic/TargetInfo.h
@@ -816,6 +816,7 @@
 struct {
   int Min;
   int Max;
+  bool isConstrained;
 } ImmRange;
 llvm::SmallSet ImmSet;
 
@@ -826,6 +827,7 @@
 : Flags(0), TiedOperand(-1), ConstraintStr(ConstraintStr.str()),
   Name(Name.str()) {
   ImmRange.Min = ImmRange.Max = 0;
+  ImmRange.isConstrained = false;
 }
 
 const std::string &getConstraintStr() const { return ConstraintStr; }
@@ -854,8 +856,9 @@
   return (Flags & CI_ImmediateConstant) != 0;
 }
 bool isValidAsmImmediate(const llvm::APInt &Value) const {
-  return (Value.sge(ImmRange.Min) && Value.sle(ImmRange.Max)) ||
- ImmSet.count(Value.getZExtValue()) != 0;
+  if (!ImmSet.empty())
+return ImmSet.count(Value.getZExtValue()) != 0;
+  return !ImmRange.isConstrained || (Value.sge(ImmRange.Min) && Value.sle(ImmRange.Max));
 }
 
 void setIsReadWrite() { Flags |= CI_ReadWrite; }
@@ -867,6 +870,7 @@
   Flags |= CI_ImmediateConstant;
   ImmRange.Min = Min;
   ImmRange.Max = Max;
+  ImmRange.isConstrained = true;
 }
 void setRequiresImmediate(llvm::ArrayRef Exacts) {
   Flags |= CI_ImmediateConstant;
@@ -879,8 +883,6 @@
 }
 void setRequiresImmediate() {
   Flags |= CI_ImmediateConstant;
-  ImmRange.Min = INT_MIN;
-  ImmRange.Max = INT_MAX;
 }
 
 /// Indicate that this is an input operand that is tied to
Index: cfe/trunk/test/Sema/inline-asm-validate-x86.c
===
--- cfe/trunk/test/Sema/inline-asm-validate-x86.c
+++ cfe/trunk/test/Sema/inline-asm-validate-x86.c
@@ -55,6 +55,7 @@
 void L(int i, int j) {
   static c

[PATCH] D58649: Fix inline assembler constraint validation

[Joerg Sonnenberger via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/TargetInfo.h:860
+  if (!ImmSet.empty())
+return ImmSet.count(Value.getZExtValue()) != 0;
+  return !ImmRange.isConstrained || (Value.sge(ImmRange.Min) && 
Value.sle(ImmRange.Max));

efriedma wrote:
> Isn't the "ImmSet.count" call still wrong?  It looks like it crashes on an 
> 128-bit immediate, and implicitly truncates a 64-bit immediate.  I guess you 
> could check `Value.isSignedIntN(32)`?
I've added that and a test case for truncation as a follow-up.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58649



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

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Can you include a patch for something like (int *)0xdeadbeef on amd64? 
That's a valid value for "n", but clearly too large for int. Thanks for looking 
at this, it is one of the two large remaining show stoppers for the asm 
constraint check.


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

https://reviews.llvm.org/D58821



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

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

The other problem is that we don't use the CFG machinery to prune dead 
branches. Consider the x86 in/out instructions: one variant takes an immediate, 
the other a register. The classic way to deal with that is something like

  static inline void outl(unsigned port, uint32_t value)
  {
if (__builtin_constant_p(port) && port < 0x100) {
  __asm volatile("outl %0,%w1" : : "a" (data), "id" (port));
} else {
 __asm volatile("outl %0,%w1" : : "a" (data), "d" (port));
}
  }

This fails with the new asm constraint checks, since the dead branch is never 
pruned. For other architectures it makes an even greater difference. The main 
reason it is a show stopper: there is no sane workaround that doesn't regress 
code quality.


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

https://reviews.llvm.org/D58821



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

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Well, that was a sample to illustrate the point. A full working (and now 
failing) example is:

  static inline void outl(unsigned port, unsigned data) {
if (__builtin_constant_p(port) && port < 0x100) {
  __asm volatile("outl %0,%w1" : : "a"(data), "n"(port));
} else {
  __asm volatile("outl %0,%w1" : : "a"(data), "d"(port));
}
  }
  
  void f(unsigned port) { outl(1, 1); }


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

https://reviews.llvm.org/D58821



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

[PATCH] D59118: creduce script for clang crashes

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

For it to be really useful for the majority of bugs, it would be nice to figure 
out automatically how to get the preprocessing step done and filter out the # 
lines afterwards. That part alone significantly cuts down the creduce time.


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

https://reviews.llvm.org/D59118



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

[PATCH] D60943: Delay diagnosing "n" constraint until after inlining

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I'm in the process of testing this, but feedback will take a bit.

On the more meaty parts of this change, I think further iterations will be 
necessary in-tree to extend this to the other constraints.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943



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

[PATCH] D52137: Added warning for unary minus used with unsigned type

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I find this warning confusing. I find a4 to be perfectly expected. IMO this 
warning should be applied only, if the effective value of the expression is not 
the same as in the modulo-n arithmetic. This means that if (-x) is explicitly 
or implicitly cast to a less wide unsigned type, it should not warn. It would 
consider a warning for the case of using (-x) if integer promotion rules makes 
it negative though. The question is, how to best patch around the warning 
though. What options does MSVC have for that? I.e. what equivalent expressions 
do not trigger this warning?


https://reviews.llvm.org/D52137



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

[PATCH] D52696: Update ifunc attribute support documentation

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I think this is still too optimistic. Full support for ifunc seems to be 
generally limited to x86. Most other architectures lack even definitions for 
anonymous ifunc relocations or support proper relaxation only in limited forms. 
That's especially annoying when looking at static linking.


Repository:
  rL LLVM

https://reviews.llvm.org/D52696



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

[PATCH] D52696: Update ifunc attribute support documentation

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Yeah, I would restrict it to just mention that it depends on the target, link 
time editor and runtime linker. Even the concrete feature set on Linux changes 
with glibc versions.


Repository:
  rL LLVM

https://reviews.llvm.org/D52696



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

[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added inline comments.



Comment at: lib/Lex/PPMacroExpansion.cpp:1460
+  for (const auto &Entry : MacroPrefixMap)
+if (Path.startswith(Entry.first))
+  return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();

Lekensteyn wrote:
> dankm wrote:
> > dankm wrote:
> > > joerg wrote:
> > > > This doesn't handle directory vs string prefix prefix correctly, does 
> > > > it? At the very least, it should have a test case :)
> > > Good catch. I expect not. But on the other hand, it's exactly what 
> > > debug-prefix-map does :)
> > > 
> > > I'll add test cases in a future review. My first goal was getting 
> > > something sort-of working.
> > There should be a test, but apparently the debug prefix map code also does 
> > this.
> > 
> > What do you think the correct behaviour should be? a string prefix, or a 
> > directory prefix?
> It should be a string prefix (like GCC)
I disagree. I consider it a bug in GCC that it is a string prefix. It's quite 
inconsistent as well.


Repository:
  rC Clang

https://reviews.llvm.org/D49466



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

[PATCH] D58592: [clang] [ToolChains/NetBSD] Support relative libc++ header path

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I'm not in favor of this. It adds overhead for the system compiler and 
generally makes the logic more complicated. This seems to be another hack 
around the fact that the driver has no clear notion of "use system runtime" vs 
"use custom runtime".


Repository:
  rC Clang

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

https://reviews.llvm.org/D58592



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

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

This version is fine with me. The only contentious part is whether it should be 
opt-in or opt-out for platforms, so getting this version in and revisiting the 
issue again later is OK from my perspective.


https://reviews.llvm.org/D34158



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

[PATCH] D37302: [Headers] Define *_HAS_SUBNORM for FLT, DBL, LDBL

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

So what about targets that don't support subnormals? I'm moderately sure ARM 
falls into this category given the right phase of the moon.


https://reviews.llvm.org/D37302



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

[PATCH] D37954: Expand absolute system header paths when using -MD depfiles

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

The comments at the very least are misleading. Resolving the path doesn't 
necessary shorten the string at all, in many cases it will be longer. I'm 
really not sure if clang should resolve symlinks like this as it can be quite 
surprising.


https://reviews.llvm.org/D37954



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

[PATCH] D37954: Expand absolute system header paths when using -MD depfiles

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

ninja is not the only consumer of this. For human inspection, it can be quite 
surprising to see symlinks resolved, i.e. /usr/include/machine on NetBSD. Build 
systems have different ideas on whether they want absolute resolved paths or 
not, so it seems like ninja should be able to cope with either format. This is 
a lossy transformation, so I'm somewhat reluctant to agree with it.


https://reviews.llvm.org/D37954



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

[PATCH] D37954: Try to shorten system header paths when using -MD depfiles

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Well, the background for the use of the option in NetBSD is related to inducted 
differences in reproducable builds. From that perspective, it is even worth to 
sometimes shorten the dependency and sometimes not.


https://reviews.llvm.org/D37954



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

[PATCH] D60748: Fix i386 struct and union parameter alignment

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:1501
+//
+// Exclude other System V OS (e.g Darwin, PS4 and FreeBSD) since we don't
+// want to spend any effort dealing with the ramifications of ABI breaks.

krytarowski wrote:
> Darwin and BSD are not System V.
> 
> CC: @joerg @mgorny for NetBSD. Do we need to do something here?
It's a misnomer. The ABI standard for i386 was the SysV ABI before GNU decided 
to silently break the stack alignment and calling it the new ABI. That said, 
I'm not sure how much copy-by-value of vector types actually happens and that's 
the only situation affected by this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60748



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

[PATCH] D59744: Fix i386 ABI "__m64" type bug

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I think MMX code is obscure enough at this point that it doesn't matter much 
either way. Even less across DSO boundaries. That's why I don't really care 
either way.


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

https://reviews.llvm.org/D59744



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

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

You lost the changes to lib/Sema/SemaStmtAsm.cpp to actually do the delaying 
for immediate operands?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943



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

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I understand, but the current version just doesn't work anyway to delay it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943



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

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

The combination of D60942 , D06943 and D65280 
 solves the problems for me on all targets I 
have.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

This doesn't seem a reasonable approach at all:

(1) It breaks cross-linking.
(2) It is not correct for any target architecture, e.g. /usr/local/lib 
certainly doesn't belong on this list and /lib doesn't either.
(3) The correct list depends not only on the target architecture, but also the 
active emulation.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

In D56215#1344279 , @krytarowski wrote:

> In D56215#1344183 , @joerg wrote:
>
> > This doesn't seem a reasonable approach at all:
> >
> > (1) It breaks cross-linking.
> >  (2) It is not correct for any target architecture, e.g. /usr/local/lib 
> > certainly doesn't belong on this list and /lib doesn't either.
> >  (3) The correct list depends not only on the target architecture, but also 
> > the active emulation.
>
>
> Is it acceptable to pass all the paths through configure/build phase of lld? 
> It's done this way in GNU ld in the NetBSD distribution. If we need or want 
> to hardcode all the specific paths it will be harder to maintain the proper 
> list and behavior inside lld.


I don't think that would be better either. The main point is that it needs a 
lot more architectural knowledge than shown in the path. I would expect e.g. 
Linux distros have a similar problem nowadays.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Talking from the perspective of having had to deal with thousands of packages 
in pkgsrc over the years: it is naive to believe that there isn't a lot of 
software that calls the linker directly for various reasons. As such, it is 
very important to have a useful configuration in the linker by default. Such a 
configuration is by its very nature target specific. This doesn't mean it can't 
be done in a cross-compiler friendly way, on the contrary. Over the years 
NetBSD has been pushing its toolchain to be as similar for the native build and 
a cross-target as reasonable possible. Modulo the build time choices in the 
config.h sense, the only difference between the native and cross tools is the 
built-in default of the former.

Clang works extremely well in that regard and perfectly supports a universal 
toolchain. LLD should as well. Consistent behavior of ELF across different OS 
is a fiction, as such some OS and/or CPU family specific defaults are naturally 
different. This can be grouped in general:
(1) The target operating system specified either from the builtin default, the 
name of the tool (triple-ld), an option like clang's --target or if desirable 
the name of the emulation when specified. I don't have a strong reason for 
supporting the last as it is typically not unique even with binutils and more a 
case of historic legacy than useful feature. For the perspective of supporting 
both native and cross toolchains, the first three options are enough and when 
using the compiler driver, the first two will generally work fine.
(2) The target CPU family. While it can be partially guessed from the object 
files, there are fun expects. ARM instruction encodings are one of the 
pecularities a linker has to deal with for example. Is BX a valid jump 
instruction?
(3) Whether the target CPU family is the primary ABI. This can generally not be 
determined from the object files and can affect the default search paths. Hard 
vs soft floating point is a good example here. Other cases are easier like 
linking i386 binaries on x86_64. N32 vs N64 on MIPS is more difficult again. 
This is a bit tricky in that it often can be drived only from the emulation 
name.

In terms of architecture, it doesn't need much, but it needs some thought. 
Identifying the target OS is likely the easiest. Most of the rest is OS 
specific adjustment. Having access to the binary name and the the arguments 
should be enough though. Doing it properly avoids the fun of having to patch a 
lot of software without costing that much in terms of code.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Thanks, this looks like a good starting point.


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

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added inline comments.



Comment at: ELF/Driver.cpp:779
+// TODO: verify the triple somehow?
+Config->TargetTriple = llvm::Triple(Prefix);
+  }

See ToolChain::getTargetAndModeFromProgramName in clang.


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

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

@ruiu: No, it is exactly what you want, since it allows you to point lld into 
the normal sysroot. Cross-compiling is the default case for the NetBSD 
toolchain.


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

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added inline comments.



Comment at: ELF/Driver.cpp:381
+}
+Config->SearchPaths.push_back("/usr/lib");
+  }

Those need to be sysroot-relative of course.


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

https://reviews.llvm.org/D56215



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

[PATCH] D67638: Improve __builtin_constant_p lowering

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg created this revision.
joerg added a reviewer: rsmith.
Herald added subscribers: kristina, kbarton, nemanjai.
Herald added a project: clang.
Herald added a subscriber: wuzish.

__builtin_constant_p used to be short-cut evaluated to false when building with 
-O0. This is undesirable as it means that constant folding in the front-end can 
give different results than folding in the back-end.  It can also create 
conditional branches on constant conditions that don't get folded away. With 
the improvements to the llvm.is.constant handling on the LLVM side, the 
short-cut is no longer useful.

Adjust various codegen tests to not depend on the optimizations.


Repository:
  rC Clang

https://reviews.llvm.org/D67638

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/builtin-constant-p.c
  test/CodeGen/ppc-emmintrin.c
  test/CodeGen/ppc-tmmintrin.c

Index: test/CodeGen/ppc-tmmintrin.c
===
--- test/CodeGen/ppc-tmmintrin.c
+++ test/CodeGen/ppc-tmmintrin.c
@@ -108,7 +108,8 @@
 // CHECK: store <2 x i64> [[REG61]], <2 x i64>* [[REG65:[0-9a-zA-Z_%.]+]], align 16
 // CHECK-NEXT: store <2 x i64> [[REG62]], <2 x i64>* [[REG66:[0-9a-zA-Z_%.]+]], align 16
 // CHECK-NEXT: store i32 [[REG63]], i32* [[REG64:[0-9a-zA-Z_%.]+]], align 4
-// CHECK-NEXT: br i1 false, label %[[REG67:[0-9a-zA-Z_%.]+]], label %[[REG68:[0-9a-zA-Z_%.]+]]
+// CHECK: %[[REG64b:[0-9a-zA-Z_%.]+]] = call i1 @llvm.is.constant.i32(i32 %0)
+// CHECK-NEXT: br i1 %[[REG64b]], label %[[REG67:[0-9a-zA-Z_%.]+]], label %[[REG68:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG67]]:
 // CHECK-NEXT: [[REG69:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG64]], align 4
Index: test/CodeGen/ppc-emmintrin.c
===
--- test/CodeGen/ppc-emmintrin.c
+++ test/CodeGen/ppc-emmintrin.c
@@ -231,7 +231,9 @@
 // CHECK-NEXT: br i1 [[REG147]], label %[[REG148:[0-9a-zA-Z_%.]+]], label %[[REG149:[0-9a-zA-Z_%.]+]]
 // CHECK: [[REG148]]:
 
-// CHECK-LE-NEXT: br i1 false, label %[[REG150:[0-9a-zA-Z_%.]+]], label %[[REG151:[0-9a-zA-Z_%.]+]]
+// CHECK-LE-NEXT: load
+// CHECK-LE-NEXT: call i1 @llvm.is.constant
+// CHECK-LE-NEXT: br i1 %[[REG1896a:[0-9a-zA-Z_%.]+]], label %[[REG150:[0-9a-zA-Z_%.]+]], label %[[REG151:[0-9a-zA-Z_%.]+]]
 // CHECK-LE: [[REG150]]:
 // CHECK-LE: [[REG152:[0-9a-zA-Z_%.]+]] = load <2 x i64>, <2 x i64>* [[REG143]], align 16
 // CHECK-LE-NEXT: [[REG153:[0-9a-zA-Z_%.]+]] = bitcast <2 x i64> [[REG152]] to <16 x i8>
@@ -2326,7 +2328,9 @@
 // CHECK-NEXT: br i1 [[REG1559]], label %[[REG1560:[0-9a-zA-Z_%.]+]], label %[[REG1557:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1560]]:
-// CHECK-NEXT: br i1 false, label %[[REG1561:[0-9a-zA-Z_%.]+]], label %[[REG1562:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+// CHECK-NEXT: br i1 %[[REG1561a:[0-9a-zA-Z_%.]+]], label %[[REG1561:[0-9a-zA-Z_%.]+]], label %[[REG1562:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1561]]:
 // CHECK-NEXT: [[REG1563:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG1552]], align 4
@@ -2369,7 +2373,9 @@
 // CHECK-NEXT: br i1 [[REG1587]], label %[[REG1588:[0-9a-zA-Z_%.]+]], label %[[REG1585:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1588]]:
-// CHECK-NEXT: br i1 false, label %[[REG1589:[0-9a-zA-Z_%.]+]], label %[[REG1590:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+// CHECK-NEXT: br i1 %{{[0-9a-zA-Z_%.]+}}, label %[[REG1589:[0-9a-zA-Z_%.]+]], label %[[REG1590:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1589]]:
 // CHECK-NEXT: [[REG1591:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG1580]], align 4
@@ -2416,7 +2422,9 @@
 // CHECK-NEXT: br i1 [[REG1617]], label %[[REG1618:[0-9a-zA-Z_%.]+]], label %[[REG1615:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1618]]:
-// CHECK-NEXT: br i1 false, label %[[REG1619:[0-9a-zA-Z_%.]+]], label %[[REG1620:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+// CHECK-NEXT: br i1 %{{[0-9a-zA-Z_%.]+}}, label %[[REG1619:[0-9a-zA-Z_%.]+]], label %[[REG1620:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1619]]:
 // CHECK-NEXT: [[REG1621:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG1610]], align 4
@@ -2563,7 +2571,9 @@
 // CHECK-NEXT: br i1 [[REG1712]], label %[[REG1713:[0-9a-zA-Z_%.]+]], label %[[REG1714:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1713]]:
-// CHECK-NEXT: br i1 false, label %[[REG1715:[0-9a-zA-Z_%.]+]], label %[[REG1716:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+// CHECK-NEXT: br i1 %[[REG1715a:[0-9a-zA-Z_%.]+]], label %[[REG1715:[0-9a-zA-Z_%.]+]], label %[[REG1716:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1715]]:
 // CHECK-NEXT: [[REG1717:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG1709]], align 4
@@ -2601,7 +2611,9 @@
 // CHECK-NEXT: br i1 [[REG1737]], label %[[REG1738:[0-9a-zA-Z_%.]+]], label %[[REG1739:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1738]]:
-// CHECK-NEXT: br i1 false, label %[[REG1740:[0-9a-zA-Z_%.]+]], label %[[REG1741:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+//

[PATCH] D67638: Improve __builtin_constant_p lowering

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67638



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

[PATCH] D41240: [Solaris] __float128 is supported on Solaris/x86

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Things are different for a libgcc-based toolchain and a compiler-rt based 
toolchain.


Repository:
  rL LLVM

https://reviews.llvm.org/D41240



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

[PATCH] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I'm back to the point where I can't reproduce the problem :( Can we start 
providing an actual failing test case? It's annoying to debug a problem when 
you can't reproduce it.


https://reviews.llvm.org/D38680



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

[PATCH] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size

[Joerg Sonnenberger via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL337312: The semantics of DW_CFA_GNU_args_size have changed 
subtile over the (authored by joerg, committed by ).
Herald added subscribers: llvm-commits, christof.

Changed prior to commit:
  https://reviews.llvm.org/D38680?vs=150049&id=155943#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38680

Files:
  libunwind/trunk/src/UnwindCursor.hpp
  libunwind/trunk/src/libunwind.cpp


Index: libunwind/trunk/src/UnwindCursor.hpp
===
--- libunwind/trunk/src/UnwindCursor.hpp
+++ libunwind/trunk/src/UnwindCursor.hpp
@@ -1411,8 +1411,6 @@
 this->setInfoBasedOnIPRegister(true);
 if (_unwindInfoMissing)
   return UNW_STEP_END;
-if (_info.gp)
-  setReg(UNW_REG_SP, getReg(UNW_REG_SP) + _info.gp);
   }
 
   return result;
Index: libunwind/trunk/src/libunwind.cpp
===
--- libunwind/trunk/src/libunwind.cpp
+++ libunwind/trunk/src/libunwind.cpp
@@ -188,8 +188,20 @@
 co->setReg(regNum, (pint_t)value);
 // specical case altering IP to re-find info (being called by personality
 // function)
-if (regNum == UNW_REG_IP)
+if (regNum == UNW_REG_IP) {
+  unw_proc_info_t info;
+  // First, get the FDE for the old location and then update it.
+  co->getInfo(&info);
   co->setInfoBasedOnIPRegister(false);
+  // If the original call expects stack adjustment, perform this now.
+  // Normal frame unwinding would have included the offset already in the
+  // CFA computation.
+  // Note: for PA-RISC and other platforms where the stack grows up,
+  // this should actually be - info.gp. LLVM doesn't currently support
+  // any such platforms and Clang doesn't export a macro for them.
+  if (info.gp)
+co->setReg(UNW_REG_SP, co->getReg(UNW_REG_SP) + info.gp);
+}
 return UNW_ESUCCESS;
   }
   return UNW_EBADREG;


Index: libunwind/trunk/src/UnwindCursor.hpp
===
--- libunwind/trunk/src/UnwindCursor.hpp
+++ libunwind/trunk/src/UnwindCursor.hpp
@@ -1411,8 +1411,6 @@
 this->setInfoBasedOnIPRegister(true);
 if (_unwindInfoMissing)
   return UNW_STEP_END;
-if (_info.gp)
-  setReg(UNW_REG_SP, getReg(UNW_REG_SP) + _info.gp);
   }
 
   return result;
Index: libunwind/trunk/src/libunwind.cpp
===
--- libunwind/trunk/src/libunwind.cpp
+++ libunwind/trunk/src/libunwind.cpp
@@ -188,8 +188,20 @@
 co->setReg(regNum, (pint_t)value);
 // specical case altering IP to re-find info (being called by personality
 // function)
-if (regNum == UNW_REG_IP)
+if (regNum == UNW_REG_IP) {
+  unw_proc_info_t info;
+  // First, get the FDE for the old location and then update it.
+  co->getInfo(&info);
   co->setInfoBasedOnIPRegister(false);
+  // If the original call expects stack adjustment, perform this now.
+  // Normal frame unwinding would have included the offset already in the
+  // CFA computation.
+  // Note: for PA-RISC and other platforms where the stack grows up,
+  // this should actually be - info.gp. LLVM doesn't currently support
+  // any such platforms and Clang doesn't export a macro for them.
+  if (info.gp)
+co->setReg(UNW_REG_SP, co->getReg(UNW_REG_SP) + info.gp);
+}
 return UNW_ESUCCESS;
   }
   return UNW_EBADREG;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49480: Haiku: support for secondary arch

[Joerg Sonnenberger via Phabricator via cfe-commits]

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

This is absolutely not how the clang driver is supposed to work. No conditional 
compilation.


Repository:
  rC Clang

https://reviews.llvm.org/D49480



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

[PATCH] D49482: Haiku: add a test for haiku driver

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

It seems to miss most of the interesting checks, i.e. crt files. Compare with 
any of the entries on netbsd.c for example.


Repository:
  rC Clang

https://reviews.llvm.org/D49482



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

[PATCH] D49481: Haiku: Enable thread-local storage and disable PIE by default

[Joerg Sonnenberger via Phabricator via cfe-commits]

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

Both needs a test case :)


Repository:
  rC Clang

https://reviews.llvm.org/D49481



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

[PATCH] D49466: Initial implementation of -fmacro-prefix-mapand -ffile-prefix-map

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:609
 static void addDebugPrefixMapArg(const Driver &D, const ArgList &Args, 
ArgStringList &CmdArgs) {
+  for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ)) {
+StringRef Map = A->getValue();

I find it confusing that `-ffile_prefix_map` implies `-fdebug-prefix-map`. I'm 
not sure that is desirable in every case. It seems better to have a combined 
option that explicitly does both.



Comment at: lib/Driver/ToolChains/Clang.cpp:612
+if (Map.find('=') == StringRef::npos)
+  D.Diag(diag::err_drv_invalid_argument_to_fdebug_prefix_map) << Map;
+else

I'd prefer the bailout style here, i.e. `if (...) { D.diag(...); continue }`



Comment at: lib/Driver/ToolChains/Clang.cpp:628
+/// Add a CC1 option to specify the macro file path prefix map.
+static void addMacroPrefixMapArg(const Driver &D, const ArgList &Args, 
ArgStringList &CmdArgs) {
+  for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ)) {

erichkeane wrote:
> See advice above.
> 
> Additionally/alternatively, I wonder if these two functions could be 
> trivially combined.
Or at least the for loop could be refactored into a small helper function that 
takes the option name, output option and error as argument.



Comment at: lib/Lex/PPMacroExpansion.cpp:1460
+  for (const auto &Entry : MacroPrefixMap)
+if (Path.startswith(Entry.first))
+  return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();

This doesn't handle directory vs string prefix prefix correctly, does it? At 
the very least, it should have a test case :)


Repository:
  rC Clang

https://reviews.llvm.org/D49466



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

[PATCH] D68410: [AttrDocs] document always_inline

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I wonder if we should actually enumerate evil here, i.e. give the situations in 
which inlining actually fails. As mentioned on IRC, I wonder if we shouldn't 
aim for the stronger semantics and at least warn by default of any situation 
that prevents always_inline from doing its job.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68410



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

[PATCH] D67638: Improve __builtin_constant_p lowering

[Joerg Sonnenberger via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG529f4ed401ea: Improve __builtin_constant_p lowering 
(authored by joerg).

Changed prior to commit:
  https://reviews.llvm.org/D67638?vs=220398&id=224796#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67638

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtin-constant-p.c
  clang/test/CodeGen/ppc-emmintrin.c
  clang/test/CodeGen/ppc-tmmintrin.c

Index: clang/test/CodeGen/ppc-tmmintrin.c
===
--- clang/test/CodeGen/ppc-tmmintrin.c
+++ clang/test/CodeGen/ppc-tmmintrin.c
@@ -108,7 +108,8 @@
 // CHECK: store <2 x i64> [[REG61]], <2 x i64>* [[REG65:[0-9a-zA-Z_%.]+]], align 16
 // CHECK-NEXT: store <2 x i64> [[REG62]], <2 x i64>* [[REG66:[0-9a-zA-Z_%.]+]], align 16
 // CHECK-NEXT: store i32 [[REG63]], i32* [[REG64:[0-9a-zA-Z_%.]+]], align 4
-// CHECK-NEXT: br i1 false, label %[[REG67:[0-9a-zA-Z_%.]+]], label %[[REG68:[0-9a-zA-Z_%.]+]]
+// CHECK: %[[REG64b:[0-9a-zA-Z_%.]+]] = call i1 @llvm.is.constant.i32(i32 %0)
+// CHECK-NEXT: br i1 %[[REG64b]], label %[[REG67:[0-9a-zA-Z_%.]+]], label %[[REG68:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG67]]:
 // CHECK-NEXT: [[REG69:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG64]], align 4
Index: clang/test/CodeGen/ppc-emmintrin.c
===
--- clang/test/CodeGen/ppc-emmintrin.c
+++ clang/test/CodeGen/ppc-emmintrin.c
@@ -231,7 +231,9 @@
 // CHECK-NEXT: br i1 [[REG147]], label %[[REG148:[0-9a-zA-Z_%.]+]], label %[[REG149:[0-9a-zA-Z_%.]+]]
 // CHECK: [[REG148]]:
 
-// CHECK-LE-NEXT: br i1 false, label %[[REG150:[0-9a-zA-Z_%.]+]], label %[[REG151:[0-9a-zA-Z_%.]+]]
+// CHECK-LE-NEXT: load
+// CHECK-LE-NEXT: call i1 @llvm.is.constant
+// CHECK-LE-NEXT: br i1 %[[REG1896a:[0-9a-zA-Z_%.]+]], label %[[REG150:[0-9a-zA-Z_%.]+]], label %[[REG151:[0-9a-zA-Z_%.]+]]
 // CHECK-LE: [[REG150]]:
 // CHECK-LE: [[REG152:[0-9a-zA-Z_%.]+]] = load <2 x i64>, <2 x i64>* [[REG143]], align 16
 // CHECK-LE-NEXT: [[REG153:[0-9a-zA-Z_%.]+]] = bitcast <2 x i64> [[REG152]] to <16 x i8>
@@ -2326,7 +2328,9 @@
 // CHECK-NEXT: br i1 [[REG1559]], label %[[REG1560:[0-9a-zA-Z_%.]+]], label %[[REG1557:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1560]]:
-// CHECK-NEXT: br i1 false, label %[[REG1561:[0-9a-zA-Z_%.]+]], label %[[REG1562:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+// CHECK-NEXT: br i1 %[[REG1561a:[0-9a-zA-Z_%.]+]], label %[[REG1561:[0-9a-zA-Z_%.]+]], label %[[REG1562:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1561]]:
 // CHECK-NEXT: [[REG1563:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG1552]], align 4
@@ -2369,7 +2373,9 @@
 // CHECK-NEXT: br i1 [[REG1587]], label %[[REG1588:[0-9a-zA-Z_%.]+]], label %[[REG1585:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1588]]:
-// CHECK-NEXT: br i1 false, label %[[REG1589:[0-9a-zA-Z_%.]+]], label %[[REG1590:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+// CHECK-NEXT: br i1 %{{[0-9a-zA-Z_%.]+}}, label %[[REG1589:[0-9a-zA-Z_%.]+]], label %[[REG1590:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1589]]:
 // CHECK-NEXT: [[REG1591:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG1580]], align 4
@@ -2416,7 +2422,9 @@
 // CHECK-NEXT: br i1 [[REG1617]], label %[[REG1618:[0-9a-zA-Z_%.]+]], label %[[REG1615:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1618]]:
-// CHECK-NEXT: br i1 false, label %[[REG1619:[0-9a-zA-Z_%.]+]], label %[[REG1620:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+// CHECK-NEXT: br i1 %{{[0-9a-zA-Z_%.]+}}, label %[[REG1619:[0-9a-zA-Z_%.]+]], label %[[REG1620:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1619]]:
 // CHECK-NEXT: [[REG1621:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG1610]], align 4
@@ -2563,7 +2571,9 @@
 // CHECK-NEXT: br i1 [[REG1712]], label %[[REG1713:[0-9a-zA-Z_%.]+]], label %[[REG1714:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1713]]:
-// CHECK-NEXT: br i1 false, label %[[REG1715:[0-9a-zA-Z_%.]+]], label %[[REG1716:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+// CHECK-NEXT: br i1 %[[REG1715a:[0-9a-zA-Z_%.]+]], label %[[REG1715:[0-9a-zA-Z_%.]+]], label %[[REG1716:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1715]]:
 // CHECK-NEXT: [[REG1717:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG1709]], align 4
@@ -2601,7 +2611,9 @@
 // CHECK-NEXT: br i1 [[REG1737]], label %[[REG1738:[0-9a-zA-Z_%.]+]], label %[[REG1739:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1738]]:
-// CHECK-NEXT: br i1 false, label %[[REG1740:[0-9a-zA-Z_%.]+]], label %[[REG1741:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+// CHECK-NEXT: br i1 %[[REG1738:[0-9a-zA-Z_%.]+]], label %[[REG1740:[0-9a-zA-Z_%.]+]], label %[[REG1741:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1740]]:
 // CHECK-NEXT: [[REG1742:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG1734]], align 4
@@ -2741,7 +2753,9 @@
 // CHECK-NEXT: [[REG1837:

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I do agree with the feature request, but I'm not sure about the implementation. 
It doesn't seem to work well with the cross-compiling support in the driver as 
clearly shown by the amount of tests that need patching. Is cross-compiling a 
concern for you at all? Otherwise I would suggest going a slightly different 
route and use default values iff no "-target" or "target-command" mechanism is 
active. Might need a way to temporarily disable unused flag warnings, but that 
way would be pretty much toolchain independent?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80300



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

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I see that more as a short-coming in the existing DEFAULT_SYSROOT behavior and 
less an argument for making more cases like it. So the general idea is that for 
turnkey toolchains,
we want to allow customizing the "default" target of the toolchain to hard-code 
options like --sysroot, --target, -Wl,-rpath etc. Those are all related, so 
when using a different target, they no longer make sense. One way to deal with 
all those options in a consistent manner is hook into the logic for determining 
the current target, if none is explicitly specified on the command line or 
implicit from the executable name, then prepend some additional flags on the 
command line based on some cmake variable or so. This flags shouldn't trigger 
the unused argument warnings, so you can always pass -Wl,-rpath, --sysroot etc, 
independent of whether the compiler is in preprocessing / compile / assemble / 
link mode. That seems to be a more general and systematic approach than adding 
many additional build-time options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80300



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

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I don't agree with the justification at all, but it also seems that noone else 
cares about the build option creep here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80300



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

[PATCH] D73245: Avoid using std::max_align_t in pre-C++11 mode

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

ping2

Louis, did I answer your questions?


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

https://reviews.llvm.org/D73245



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

[PATCH] D73245: Avoid using std::max_align_t in pre-C++11 mode

[Joerg Sonnenberger via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Revision".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG98f77828a98f: Avoid using std::max_align_t in pre-C++11 mode 
(authored by joerg).
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

Changed prior to commit:
  https://reviews.llvm.org/D73245?vs=250937&id=254949#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73245

Files:
  libcxx/include/cstddef
  libcxx/include/stddef.h
  
libcxx/test/libcxx/experimental/memory/memory.resource.adaptor/memory.resource.adaptor.mem/db_deallocate.pass.cpp
  libcxx/test/libcxx/language.support/support.dynamic/libcpp_deallocate.sh.cpp
  libcxx/test/std/containers/sequences/array/array.data/data.pass.cpp
  libcxx/test/std/containers/sequences/array/array.data/data_const.pass.cpp
  libcxx/test/std/containers/sequences/array/size_and_alignment.pass.cpp
  libcxx/test/std/depr/depr.c.headers/stddef_h.pass.cpp
  libcxx/test/std/language.support/support.types/max_align_t.pass.cpp
  
libcxx/test/std/utilities/meta/meta.trans/meta.trans.other/aligned_storage.pass.cpp

Index: libcxx/test/std/utilities/meta/meta.trans/meta.trans.other/aligned_storage.pass.cpp
===
--- libcxx/test/std/utilities/meta/meta.trans/meta.trans.other/aligned_storage.pass.cpp
+++ libcxx/test/std/utilities/meta/meta.trans/meta.trans.other/aligned_storage.pass.cpp
@@ -17,6 +17,20 @@
 #include// for std::max_align_t
 #include "test_macros.h"
 
+// The following tests assume naturally aligned types exist
+// up to 64bit (double). For larger types, max_align_t should
+// give the correct alignment. For pre-C++11 testing, only
+// the lower bound is checked.
+
+#if TEST_STD_VER < 11
+struct natural_alignment {
+long t1;
+long long t2;
+double t3;
+long double t4;
+};
+#endif
+
 int main(int, char**)
 {
 {
@@ -250,9 +264,6 @@
 static_assert(std::alignment_of::value == 8, "");
 static_assert(sizeof(T1) == 16, "");
 }
-// Use alignof(std::max_align_t) below to find the max alignment instead of
-// hardcoding it, because it's different on different platforms.
-// (For example 8 on arm and 16 on x86.)
 {
 typedef std::aligned_storage<16>::type T1;
 #if TEST_STD_VER > 11
@@ -260,8 +271,15 @@
 #endif
 static_assert(std::is_trivial::value, "");
 static_assert(std::is_standard_layout::value, "");
-static_assert(std::alignment_of::value == TEST_ALIGNOF(std::max_align_t),
-  "");
+#if TEST_STD_VER >= 11
+const size_t alignment = TEST_ALIGNOF(std::max_align_t) > 16 ?
+16 : TEST_ALIGNOF(std::max_align_t);
+static_assert(std::alignment_of::value == alignment, "");
+#else
+static_assert(std::alignment_of::value >=
+  TEST_ALIGNOF(natural_alignment), "");
+static_assert(std::alignment_of::value <= 16, "");
+#endif
 static_assert(sizeof(T1) == 16, "");
 }
 {
@@ -271,9 +289,17 @@
 #endif
 static_assert(std::is_trivial::value, "");
 static_assert(std::is_standard_layout::value, "");
-static_assert(std::alignment_of::value == TEST_ALIGNOF(std::max_align_t),
-  "");
-static_assert(sizeof(T1) == 16 + TEST_ALIGNOF(std::max_align_t), "");
+#if TEST_STD_VER >= 11
+const size_t alignment = TEST_ALIGNOF(std::max_align_t) > 16 ?
+16 : TEST_ALIGNOF(std::max_align_t);
+static_assert(std::alignment_of::value == alignment, "");
+static_assert(sizeof(T1) == 16 + alignment, "");
+#else
+static_assert(std::alignment_of::value >=
+  TEST_ALIGNOF(natural_alignment), "");
+static_assert(std::alignment_of::value <= 16);
+static_assert(sizeof(T1) % TEST_ALIGNOF(natural_alignment) == 0, "");
+#endif
 }
 {
 typedef std::aligned_storage<10>::type T1;
Index: libcxx/test/std/language.support/support.types/max_align_t.pass.cpp
===
--- libcxx/test/std/language.support/support.types/max_align_t.pass.cpp
+++ libcxx/test/std/language.support/support.types/max_align_t.pass.cpp
@@ -6,6 +6,8 @@
 //
 //===--===//
 
+// UNSUPPORTED: c++98, c++03
+
 #include 
 #include 
 
@@ -41,5 +43,11 @@
   "std::alignment_of::value >= "
   "std::alignment_of::value");
 
+#ifdef __STDCPP_DEFAULT_NEW_ALIGNMENT__
+static_assert(std::alignment_of::value <=
+  __STDCPP_DEFAULT_NEW_ALIGNMENT__,
+  "max_align_t alignment is no larger than new alignment");
+#endif
+
   return 0;
 }
Index: libcxx/test/std/depr/depr.c.headers/stddef_h.pass.cpp
===
--- libcxx/test/std/depr/depr.c

[PATCH] D77697: libc++: adjust modulemap for non-modular C

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

This fixes the module build of clang for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77697



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

[PATCH] D77697: libc++: adjust modulemap for non-modular C

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

@ldionne I was updating libc++ from d42baff45d9700a199982ba0ac04dbc6c6d911bb 
 and LLVM 
itself from 38aebe5c04ab4cb3695dc1bcc60b9a7b55215aff 
 to 
3d1424bc7a0e9a6f9887727e2bc93a10f50e1709 
 when it 
started failing. It likely has been present for a while.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77697



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

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Why is this fold preferable to `(X & (X-1)) == 0`? At least on all 
architectures without native population count, the binary-and based test is 
preferable and it might even be better with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077

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

[PATCH] D106577: [clang] Define __STDC_ISO_10646__

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

This patch is certainly wrong for NetBSD as the wchar_t encoding is up to the 
specific locale charset and *not* UCS-2 or UCS-4 for certain legacy encodings 
like the various shift encodings in East Asia.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106577

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

[PATCH] D106577: [clang] Define __STDC_ISO_10646__

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

In D106577#2899715 , @aaron.ballman 
wrote:

> In D106577#2899711 , @joerg wrote:
>
>> This patch is certainly wrong for NetBSD as the wchar_t encoding is up to 
>> the specific locale charset and *not* UCS-2 or UCS-4 for certain legacy 
>> encodings like the various shift encodings in East Asia.
>
> How does the value of a macro get impacted by a runtime locale?

NetBSD doesn't set the macro. And yes, this is one of the fundamental design 
issues of long char literals. Section 2 of the following now 20 year old Itojun 
paper goes into some of the problems with the assumption of a single universal 
character set:
https://www.usenix.org/legacy/publications/library/proceedings/usenix01/freenix01/full_papers/hagino/hagino.pdf
Even an encoding that embeds ISO 10646 fully and uses a flag bit to denote 
values (entirely valid as Unicode is restricted to 21bit) should not get this 
macro set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106577

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

[PATCH] D106577: [clang] Define __STDC_ISO_10646__

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

> I still don't fully understand the original comment from Joerg. The encoding 
> of `L'a'` cannot change at runtime; it's a literal whose encoding is decided 
> entirely at compile time. @joerg -- did you mean that Clang produces a 
> different literal encoding depending on the environment the host compiler is 
> running in?

It should, which is exactly why I consider non-trivial wchar_t literals 
fundamentally flawed as concept. AFAICT the only somewhat portable value is \0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106577

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

[PATCH] D121512: [Support] Change zlib::compress to return void

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121512

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

[PATCH] D123345: Treat `std::move`, `forward`, and `move_if_noexcept` as builtins.

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

As is, I think this conflicts with `-ffreestanding` assumptions or at the very 
least the spirit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123345

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

[PATCH] D123345: Treat `std::move`, `forward`, and `move_if_noexcept` as builtins.

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

The patch contains at least one user visible change that would be quite 
surprising: it is no longer possible to intentionally set a break point on 
`std::move`.

Thinking more about it, what about a slightly different implementation 
strategy? Provide a compiler built-in `__builtin_std_move`. If it is present, 
libc++ can alias it into `namespace std` using regular C++. Much of the 
name-matching and argument checking in various places disappears. The check to 
skip template instantiation can be done the same way. Over-all, it should be 
much less intrusive with the same performance benefits for an built-in-aware 
STL. It completely side-steps the question of ignoring the actual 
implementation `of std::move` in the source, because there is none.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123345

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