<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 2018-06-04 6:19 PM, David Blaikie
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAENS6EuDf9nE_UsGZLYoCVBH0uTg+a_m68c6de1oU8hmvjFmww@mail.gmail.com">
<div dir="ltr"><br>
<br>
<div class="gmail_quote">
<div dir="ltr">On Mon, Jun 4, 2018 at 11:47 AM Chandler
Carruth <<a href="mailto:chandlerc@gmail.com"
moz-do-not-send="true">chandlerc@gmail.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">
<div class="gmail_quote">
<div dir="ltr">On Mon, Jun 4, 2018 at 8:46 PM Erik
Pilkington via llvm-commits <<a
href="mailto:llvm-commits@lists.llvm.org"
target="_blank" moz-do-not-send="true">llvm-commits@lists.llvm.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<p><br>
</p>
<br>
<div
class="m_3652827206548975187m_1453370472255763381moz-cite-prefix">On
2018-06-04 2:20 PM, David Blaikie wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">+Erik who seems to be someone
interested in this API<br>
<br>
<div class="gmail_quote">
<div dir="ltr">On Mon, Jun 4, 2018 at 11:11 AM
Chandler Carruth <<a
href="mailto:chandlerc@gmail.com"
target="_blank" moz-do-not-send="true">chandlerc@gmail.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote"
style="margin:0 0 0 .8ex;border-left:1px
#ccc solid;padding-left:1ex">
<div dir="ltr">
<div class="gmail_quote">
<div dir="ltr">On Mon, Jun 4, 2018 at
8:01 PM David Blaikie via llvm-commits
<<a
href="mailto:llvm-commits@lists.llvm.org"
target="_blank"
moz-do-not-send="true">llvm-commits@lists.llvm.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote"
style="margin:0 0 0
.8ex;border-left:1px #ccc
solid;padding-left:1ex">
<div dir="ltr">(+Chandler since he had
some feedback on another layering
change anyway)<br>
<br>
</div>
<div dir="ltr">
<div class="gmail_quote">
<div dir="ltr">On Fri, Jun 1, 2018
at 2:31 PM Justin Bogner <<a
href="mailto:mail@justinbogner.com" target="_blank"
moz-do-not-send="true">mail@justinbogner.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote"
style="margin:0 0 0
.8ex;border-left:1px #ccc
solid;padding-left:1ex">David
Blaikie via llvm-commits <<a
href="mailto:llvm-commits@lists.llvm.org" target="_blank"
moz-do-not-send="true">llvm-commits@lists.llvm.org</a>>
writes:<br>
> Author: dblaikie<br>
> Date: Wed Mar 21 10:31:49
2018<br>
> New Revision: 328123<br>
><br>
> URL: <a
href="http://llvm.org/viewvc/llvm-project?rev=328123&view=rev"
rel="noreferrer"
target="_blank"
moz-do-not-send="true">http://llvm.org/viewvc/llvm-project?rev=328123&view=rev</a><br>
> Log:<br>
> Reapply Support layering
fixes.<br>
><br>
> Compiler.h is used by
Demangle (which Support depends
on) - so sink it<br>
> into Demangle to avoid a
circular dependency<br>
<br>
Sorry for seeing this a bit
late, but this fix seems very
wrong to me.<br>
<br>
> DataTypes.h is used by
llvm-c (which Support depends
on) - so sink it<br>
> into llvm-c.<br>
><br>
> DataTypes.h could probably
be fixed the other way - making
llvm-c depend<br>
> on Support instead of
Support depending on llvm-c - if
anyone feels<br>
> that's the better option,
happy to work with them on that.<br>
><br>
> I /think/ this'll address
the layering issues that
previous attempts to<br>
> commit this have triggered
in the Modules buildbot, but I
haven't been<br>
> able to reproduce that
build so can't say for sure. If
anyone's having<br>
> trouble with this - it
might be worth taking a look to
see if there's a<br>
> quick fix/something small I
missed rather than revert, but
no worries.<br>
><br>
> Copied:
llvm/trunk/include/llvm/Demangle/Compiler.h
(from r328122,
llvm/trunk/include/llvm/Support/Compiler.h)<br>
> URL: <a
href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Demangle/Compiler.h?p2=llvm/trunk/include/llvm/Demangle/Compiler.h&p1=llvm/trunk/include/llvm/Support/Compiler.h&r1=328122&r2=328123&rev=328123&view=diff"
rel="noreferrer"
target="_blank"
moz-do-not-send="true">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Demangle/Compiler.h?p2=llvm/trunk/include/llvm/Demangle/Compiler.h&p1=llvm/trunk/include/llvm/Support/Compiler.h&r1=328122&r2=328123&rev=328123&view=diff</a><br>
>
==============================================================================<br>
> ---
llvm/trunk/include/llvm/Support/Compiler.h
(original)<br>
> +++
llvm/trunk/include/llvm/Demangle/Compiler.h
Wed Mar 21 10:31:49 2018<br>
> @@ -1,4 +1,4 @@<br>
> -//===--
llvm/Support/Compiler.h -
Compiler abstraction support
--*- C++ -*-===//<br>
> +//===--
llvm/Demangle/Compiler.h -
Compiler abstraction support -*-
C++ -*-===//<br>
> //<br>
> // The
LLVM Compiler Infrastructure<br>
> //<br>
><br>
> Modified:
llvm/trunk/include/llvm/Support/Compiler.h<br>
> URL: <a
href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Compiler.h?rev=328123&r1=328122&r2=328123&view=diff"
rel="noreferrer"
target="_blank"
moz-do-not-send="true">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Compiler.h?rev=328123&r1=328122&r2=328123&view=diff</a><br>
>
==============================================================================<br>
> ---
llvm/trunk/include/llvm/Support/Compiler.h
(original)<br>
> +++
llvm/trunk/include/llvm/Support/Compiler.h
Wed Mar 21 10:31:49 2018<br>
> @@ -7,500 +7,13 @@<br>
> //<br>
>
//===----------------------------------------------------------------------===//<br>
> //<br>
> -// This file defines
several macros, based on the
current compiler. This allows<br>
> -// use of
compiler-specific features in a
way that remains portable.<br>
> +// Due to layering
constraints (Support depends on
Demangler) this is a thin<br>
> +// wrapper around the
implementation that lives in
llvm-c, though most clients<br>
> +// can/should think of
this as being provided by
Support for simplicity (not<br>
> +// many clients are aware
of their dependency on
Demangler/it's a weird place to<br>
> +// own this - but didn't
seem to justify splitting
Support into "lower support"<br>
> +// and "upper support").<br>
<br>
Demangler isn't only a "weird
place" to own this, it's a
complete break<br>
in abstraction. None of this
stuff has to do with demangler
at all, so<br>
sinking it there because
demangler happens to depend in
it is purely a<br>
hack. I feel like the fact that
we left a shim header in
Compiler.h to<br>
paper over it is pretty strong
evidence that you agree here.<br>
</blockquote>
</div>
</div>
<div dir="ltr">
<div class="gmail_quote">
<div><br>
Ish. I'm not feeling all that
principled or worried about
these layering fixes - it
doesn't feel like this breaks
anything in a huge way, though
it doesn't feel super great
either - though introducing new
libraries to support the
variations of layering that are
required might be more hassle
than its worth too.<br>
</div>
</div>
</div>
<div dir="ltr">
<div class="gmail_quote">
<blockquote class="gmail_quote"
style="margin:0 0 0
.8ex;border-left:1px #ccc
solid;padding-left:1ex">I assume
you considered moving Signals.h
(the only thing that depends on<br>
Demangle) out of Support and
rejected that idea for one
reason or<br>
another, but that's obviously
one option for doing this more
cleanly.<br>
</blockquote>
</div>
</div>
<div dir="ltr">
<div class="gmail_quote">
<div><br>
Hadn't looked at that option,
really, though... <br>
<br>
Looks like that'd have the same
problem - since Signals.h is
included from a bunch of places
(fewer than Compiler.h, but
still tens) & similarly has
nothing to do with the
Demangler.<br>
<br>
Or were you suggesting moving it
somewhere else? Where? & its
implementation
(lib/Support/Signals.cpp)
depends on several Support
headers - so it couldn't be
easily layered below Support by
the looks of it.<br>
</div>
</div>
</div>
<div dir="ltr">
<div class="gmail_quote">
<blockquote class="gmail_quote"
style="margin:0 0 0
.8ex;border-left:1px #ccc
solid;padding-left:1ex">If
that's problematic for some
reason, I think a pretty strong
argument<br>
could be made for moving
Compiler.h to Config/ - it's
header only and<br>
really only exists to paper over
differences in the build
configuration.<br>
</blockquote>
</div>
</div>
<div dir="ltr">
<div class="gmail_quote">
<div><br>
We currently don't have any
plain headers in Config - so I'm
not sure if that'd work straight
off the bat, but don't mind
moving it there - is it worth
renaming everything that
includes "Support/Compiler.h" to
include "Config/Compiler.h"
though? Does that feel
better/more right? I'm not
really sure/don't much mind.<br>
</div>
</div>
</div>
</blockquote>
<div><br>
</div>
</div>
</div>
<div dir="ltr">
<div class="gmail_quote">
<div>I don't think this really helps.</div>
<div><br>
</div>
<div>I think that if we want Demangle to
not depend on Support, we should make
it not depend on Support --> remove
all uses of constructs from Compiler.h
and directly use the language
/compiler primitives.</div>
<div><br>
</div>
<div>Fundamentally, if Demangle needs to
be "stand alone", we need to make that
100% true. If we don't then it should
just be merged into Support itself.</div>
</div>
</div>
</blockquote>
<div><br>
Yeah, I have no particular context on
why/how this library is structured - I guess
it's isolated from Support/other LLVM libs
so it can eventually be used in the
implementation of libcxxabi?<br>
</div>
</div>
</div>
</blockquote>
</div>
<div text="#000000" bgcolor="#FFFFFF"> Yep, exactly.
It is in fact already used in libcxxabi, that repo
has an identical copy of the demangler here in LLVM.
This is a somewhat awkward state of affairs, but its
the best we can do given that libcxxabi needs a
standalone demangler, and we don't want to have to
support 2 distinct demanglers in LLVM.<br>
<br>
The LLVM demangler's dependency on Compiler.h is a
weak one, we only use LLVM_UNREACHABLE and
LLVM_FALLTHROUGH. If we're having problems with
layering in Support then I think we should just
provide simple definitions for these at the top of
ItaniumDemangle.cpp.</div>
</blockquote>
<div><br>
</div>
</div>
</div>
<div dir="ltr">
<div class="gmail_quote">
<div>This at least seems like a significantly better
short or medium term state. </div>
</div>
</div>
</blockquote>
<div><br>
Fair enough - here's all the macros that'd be needed for
ItaniumDemangle.cpp I think. Reckon that's OK to duplicate
here? Should I bring over the comments too, or leave a
comment that this is all from Support/Compiler.h?<br>
</div>
</div>
</div>
</blockquote>
That seems fine to me, that comment saying that this is all from
Compiler.h would be nice.<br>
<br>
<blockquote type="cite"
cite="mid:CAENS6EuDf9nE_UsGZLYoCVBH0uTg+a_m68c6de1oU8hmvjFmww@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div>
<div><font face="monospace">#ifndef __has_feature</font></div>
<div><font face="monospace">#define __has_feature(x) 0</font></div>
<div><font face="monospace">#endif</font></div>
<div><font face="monospace"><br>
</font></div>
<div><font face="monospace">#ifndef __has_cpp_attribute</font></div>
<div><font face="monospace">#define __has_cpp_attribute(x) 0</font></div>
<div><font face="monospace">#endif</font></div>
<div><font face="monospace"><br>
</font></div>
<div><font face="monospace">#ifndef __has_attribute</font></div>
<div><font face="monospace">#define __has_attribute(x) 0</font></div>
<div><font face="monospace">#endif</font></div>
<div><font face="monospace"><br>
</font></div>
<div><font face="monospace">#ifndef __has_builtin</font></div>
<div><font face="monospace">#define __has_builtin(x) 0</font></div>
<div><font face="monospace">#endif</font></div>
<div><font face="monospace"><br>
</font></div>
<div><font face="monospace">#ifndef LLVM_GNUC_PREREQ</font></div>
<div><font face="monospace">#if defined(__GNUC__) &&
defined(__GNUC_MINOR__) &&
defined(__GNUC_PATCHLEVEL__)</font></div>
<div><font face="monospace">#define LLVM_GNUC_PREREQ(maj,
min, patch) \</font></div>
<div><font face="monospace"> ((__GNUC__ << 20) +
(__GNUC_MINOR__ << 10) + __GNUC_PATCHLEVEL__
>= \</font></div>
<div><font face="monospace"> ((maj) << 20) + ((min)
<< 10) + (patch))</font></div>
<div><font face="monospace">#elif defined(__GNUC__)
&& defined(__GNUC_MINOR__)</font></div>
<div><font face="monospace">#define LLVM_GNUC_PREREQ(maj,
min, patch) \</font></div>
<div><font face="monospace"> ((__GNUC__ << 20) +
(__GNUC_MINOR__ << 10) >= ((maj) << 20) +
((min) << 10))</font></div>
<div><font face="monospace">#else</font></div>
<div><font face="monospace">#define LLVM_GNUC_PREREQ(maj,
min, patch) 0</font></div>
<div><font face="monospace">#endif</font></div>
<div><font face="monospace">#endif</font></div>
<div><font face="monospace"><br>
</font></div>
<div><font face="monospace">#if
__has_feature(cxx_rvalue_references) ||
LLVM_GNUC_PREREQ(4, 8, 1)</font></div>
<div><font face="monospace">#define
LLVM_HAS_RVALUE_REFERENCE_THIS 1</font></div>
<div><font face="monospace">#else</font></div>
<div><font face="monospace">#define
LLVM_HAS_RVALUE_REFERENCE_THIS 0</font></div>
<div><font face="monospace">#endif</font></div>
<div><font face="monospace"><br>
</font></div>
</div>
</div>
</div>
</blockquote>
Why do we need this macro?<br>
<br>
<blockquote type="cite"
cite="mid:CAENS6EuDf9nE_UsGZLYoCVBH0uTg+a_m68c6de1oU8hmvjFmww@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div>
<div><font face="monospace">#if __has_attribute(used) ||
LLVM_GNUC_PREREQ(3, 1, 0)</font></div>
<div><font face="monospace">#define LLVM_ATTRIBUTE_USED
__attribute__((__used__))</font></div>
<div><font face="monospace">#else</font></div>
<div><font face="monospace">#define LLVM_ATTRIBUTE_USED</font></div>
<div><font face="monospace">#endif</font></div>
<div><font face="monospace"><br>
</font></div>
<div><font face="monospace">#if
__has_builtin(__builtin_unreachable) ||
LLVM_GNUC_PREREQ(4, 5, 0)</font></div>
<div><font face="monospace">#define LLVM_BUILTIN_UNREACHABLE
__builtin_unreachable()</font></div>
<div><font face="monospace">#elif defined(_MSC_VER)</font></div>
<div><font face="monospace">#define LLVM_BUILTIN_UNREACHABLE
__assume(false)</font></div>
<div><font face="monospace">#endif</font></div>
<div><font face="monospace"><br>
</font></div>
<div><font face="monospace">#if __has_attribute(noinline) ||
LLVM_GNUC_PREREQ(3, 4, 0)</font></div>
<div><font face="monospace">#define LLVM_ATTRIBUTE_NOINLINE
__attribute__((noinline))</font></div>
<div><font face="monospace">#elif defined(_MSC_VER)</font></div>
<div><font face="monospace">#define LLVM_ATTRIBUTE_NOINLINE
__declspec(noinline)</font></div>
<div><font face="monospace">#else</font></div>
<div><font face="monospace">#define LLVM_ATTRIBUTE_NOINLINE</font></div>
<div><font face="monospace">#endif</font></div>
<div><font face="monospace"><br>
</font></div>
<div><font face="monospace">#if !defined(NDEBUG) ||
defined(LLVM_ENABLE_DUMP)</font></div>
<div><font face="monospace">#define LLVM_DUMP_METHOD
LLVM_ATTRIBUTE_NOINLINE LLVM_ATTRIBUTE_USED</font></div>
<div><font face="monospace">#else</font></div>
<div><font face="monospace">#define LLVM_DUMP_METHOD
LLVM_ATTRIBUTE_NOINLINE</font></div>
<div><font face="monospace">#endif</font></div>
<div><font face="monospace"><br>
</font></div>
<div><font face="monospace">#if __cplusplus > 201402L
&& __has_cpp_attribute(fallthrough)</font></div>
<div><font face="monospace">#define LLVM_FALLTHROUGH
[[fallthrough]]</font></div>
<div><font face="monospace">#elif
__has_cpp_attribute(gnu::fallthrough)</font></div>
<div><font face="monospace">#define LLVM_FALLTHROUGH
[[gnu::fallthrough]]</font></div>
<div><font face="monospace">#elif !__cplusplus</font></div>
<div><font face="monospace">// Workaround for <a
href="http://llvm.org/PR23435" moz-do-not-send="true">llvm.org/PR23435</a>,
since clang 3.6 and below emit a spurious</font></div>
<div><font face="monospace">// error when
__has_cpp_attribute is given a scoped attribute in C
mode.</font></div>
<div><font face="monospace">#define LLVM_FALLTHROUGH</font></div>
<div><font face="monospace">#elif
__has_cpp_attribute(clang::fallthrough)</font></div>
<div><font face="monospace">#define LLVM_FALLTHROUGH
[[clang::fallthrough]]</font></div>
<div><font face="monospace">#else</font></div>
<div><font face="monospace">#define LLVM_FALLTHROUGH</font></div>
<div><font face="monospace">#endif</font></div>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF"><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div>From Rafael's original commit:<br>
<br>
<div> My current plan is:</div>
<div> </div>
<div> Commit something like this</div>
<div> Change lld to use it</div>
<div> Change lldb to use it as the
fallback</div>
<div> </div>
<div> Add a few #ifdefs so that
exactly the same file can be used in</div>
<div> libcxxabi to export
abi::__cxa_demangle.</div>
<div> </div>
<div> Once the fast demangler in lldb can
handle any names this</div>
<div> implementation can be replaced with
it and we will have the one true</div>
<div> demangler.<br>
<br>
Not sure if anyone else is pursuing that
goal now that Rafael's left the project
and/or how much we want to leave it in a
state that makes that more possible even
if no one's actively pushing in that
direction just now.<br>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<div text="#000000" bgcolor="#FFFFFF"> After skimming
the source code, the fast demangler in LLDB really
isn't anywhere close to being a complete
implementation, and has some structural problems
that would make it impossible to replace the full
demangler. The demangler in LLVM has seen a lot of
work in the past year, and is now much faster and
much safer. I think that it should be the one true
demangler in LLVM, hopefully allowing us to remove
FastDemangle.</div>
<div text="#000000" bgcolor="#FFFFFF"><br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div>
<div>- Dave</div>
<br>
</div>
<blockquote class="gmail_quote"
style="margin:0 0 0 .8ex;border-left:1px
#ccc solid;padding-left:1ex">
<div dir="ltr">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote"
style="margin:0 0 0
.8ex;border-left:1px #ccc
solid;padding-left:1ex">
<div dir="ltr">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote"
style="margin:0 0 0
.8ex;border-left:1px #ccc
solid;padding-left:1ex"> <br>
<br>
> //<br>
>
//===----------------------------------------------------------------------===//<br>
> <br>
> -#ifndef
LLVM_SUPPORT_COMPILER_H<br>
</blockquote>
</div>
</div>
</blockquote>
</div>
</div>
<div dir="ltr">
<div class="gmail_quote">
<blockquote class="gmail_quote"
style="margin:0 0 0
.8ex;border-left:1px #ccc
solid;padding-left:1ex">
_______________________________________________<br>
llvm-commits mailing list<br>
<a
href="mailto:llvm-commits@lists.llvm.org"
target="_blank"
moz-do-not-send="true">llvm-commits@lists.llvm.org</a><br>
<a
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote>
</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
<br>
</div>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org"
target="_blank" moz-do-not-send="true">llvm-commits@lists.llvm.org</a><br>
<a
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote>
</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
<br>
</body>
</html>