[patch] Emit aliases for more constructors (V2)

Rafael EspĂ­ndola rafael.espindola at gmail.com
Sun Oct 13 09:33:44 PDT 2013


> If you're running make check -j/FORCE_PARALLEL, the print out at the end
> won't be the proper summary (it'll just be whichever shard finished last).
> If you check in testsuite/gdb.sum you should have a summary at the end with
> somewhere around 21000 tests. ( see for example the end of
> http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/9540/steps/gdb-75-check/logs/dg.sum
> )

ok, I got

# of expected passes 19677
# of unexpected successes 2
# of expected failures 130
# of unknown successes 72
# of known failures 72
# of untested testcases 58
# of unsupported tests 147

with the patch and

# of expected passes            19677
# of unexpected successes       1
# of expected failures          131
# of unknown successes          72
# of known failures             72
# of untested testcases         58
# of unsupported tests          147

without. How do I track which "expected failure" switched to a
"unexpected successes"?

> +    // FIXME: Instead of outputting an alias we could just replace every use of
> +    // AliasDecl with TargetDecl.

> FWIW I tried this but it doesn't work because we emit the globals in the wrong order for RAUW.

Yes, I got to the a similar problem when trying to implement
additional optimizations (alias to external constructor/destructor).

> Timur feels strongly that we should simply directly call the base dtor when there is no complete dtor in the MS C++ ABI.  I >want to remove the hack that makes us do that currently, though.  You can see it just below the dtor emission call to >TryEmitDefinitionAsAlias.

If I understand correctly, the ABI difference is that on itanium we
need both symbols to be present in a "weak_odr destructor" but on the
MS abi we don't. On the case of linkonce_odr not having both symbols
from the start can be a useful optimization in both ABIs. I agree that
it would be nice to not produce symbols we don't need to.

Since this patch is not incorrect, it just produces an unnecessary
extra symbol, would you be OK with it as an intermediate step?

> Does "InEveryTU" assume the user is using clang or a compiler that produces similar aliases (gcc)?  That doesn't seem very good.

No, with different compilers we get different names for the COMDATs.
If we were to link gcc and clang produced .o files, the linker will
end up keeping one copy of the comdat from clang and one from gcc,
which is wasteful but correct since the symbols is still uniqued, one
of the comdats is just dead code then.

A very long term todo is to add comdat support to the llvm IR. With
that we should be able to produce the same names as gcc for a small
extra optimization in environments that use both compilers.

A new version of the patch is attached. It has the testcase that was
breaking the gdb testsuite before and a small simplification (no
changes to CodeGenModule.cpp).

Cheers,
Rafael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.patch
Type: application/octet-stream
Size: 11211 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131013/dbf508a3/attachment.obj>


More information about the cfe-commits mailing list