[PATCH] [inline-asm] Fix scope of assembly macros

[Reid Kleckner]

On Fri, Dec 26, 2014 at 9:47 AM, Sergey Dmitrouk <sdmitrouk at accesssoftek.com
> wrote:

> @rnk:
>
> > We could also change AsmPrinter to undefine any macros introduce with
> .macro at the end of an asm blob, but still have Clang ask for the
> expansion.
>
>
> Couldn't understand what you mean for a while, looks like I didn't
> describe well
> what the patch does, sorry.  Undefining anything would make sense if
> functions
> share same context, which is still not the case (macros defined in one
> function
> are not visible in any other function).  I'm not sure if sharing
> declarations is
> safe even without LTO as it depends on order of functions.  So another
> variant
> (next paragraph) wouldn't affect lazy linking.


To me the bug is not that macros aren't carried over into the next asm
blob, it's that they are carried over if you use -no-integrated-as and are
not carried if you don't. That inconsistency is the bug.

One way to fix the inconsistency would be to have LLVM's AsmPrinter
explicitly undef macros to prevent asm blobs from escaping macros.

> We could have LLVM's LTO infrastructure expand .macro prior to linking,
> but that could kill lazy linking.
>
>
> Tried that, the problem is with determining target machine.  Correct target
> detection requires all modules to be linked together, linking them requires
> macro expansion, macro expansion requires target detection (loop).
>

This is surprising to me. Typically we know the target up front, at least
well enough to pick between the ARM or X86 asm parsers, if not all the
subtarget features.

Another option would be to define module-level macros at the top of all
> functions, but it has quite a lot of overhead and requires parsing to
> separate macros from everything else.


> > For LTO, what if we make Clang responsible for using the same asm
> context, and have it expand macros before creating the IR?
>
>
> Couldn't do this before creating the IR, need Target to parse inline asm.
> Instead wrote a pass, which expands inline asm before module is emitted.
> Please take a look and let me know what you think:
> http://reviews.llvm.org/P80


Nice! I guess one issue is that if you have code like this bad things
happen:

asm (".macro a ...")
void f() {
  asm ("use macro a");
}
asm (".undef a ...")

I have also discovered that it's easy to make two files that compile fine
> without LTO, but fail when it's used.  The issue is that module-level asm
> is
> just concatenated, which might cause conflicts.  This pass makes situation
> somewhat better as it removes module-level macro definitions.
>
> > We would still have the problem that llc's .s emission would produce
> different object files than it produces directly.
>
> I'm not sure if it's a problem, e.g. converting IR->BC->IR can change the
> code
> because of constant folding (at least, there might be more cases), which
> is in
> my opinion way more unexpected than LTO vs. non-LTO output differences.
>

Roundtripping through bitcode also kills use-list order unless you
explicitly ask to retain it. The constant folding issue just sounds like a
bug.


> In general, I don't think that forbidding anything like this is a good
> idea.  I
> don't see any good reason that would justify such decision, in fact such
> artificial constraints seem to be against users.  On the other hand,
> supporting
> every possible use case isn't the best choice as well.  I guess something
> like
> what we have here (module-level macros are visible in functions + each
> function
> has its own scope) might be a compromise that should work fine with most
> of code
> bases and don't harm compiler.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150122/13511acf/attachment.html>