LLVM 3.4.1 - Testing Phase

Done, thanks!

--renato

Hi Jiangning,

This commit that you requested be merged into the 3.4 branch has caused a regression in
the MultiSource/Applications/sgefa test:

Hi all,
   My test results for 3.4.1 on powerpc-darwin8 look good (cmake-shared built with targets: PowerPC;ARM;X86).
The 'powerpc-darwin8-rel-3.4' branch has merged the latest 'release_34' branch (github/fangism/{llvm,clang}).

release-3.4.1 RC test logs:
http://www.csl.cornell.edu/~fang/sw/llvm/logs/llvm-release-3.4.1-RC1b-O2-powerpc-darwin8-g++-4.0.1-debug-check.log.bz2
http://www.csl.cornell.edu/~fang/sw/llvm/logs/clang-release-3.4.1-RC1b-O2-powerpc-darwin8-g++-4.0.1-debug-check.log.bz2

trees for 3.4.1:
https://github.com/fangism/llvm/tree/powerpc-darwin8-rel-3.4
https://github.com/fangism/clang/tree/powerpc-darwin8-rel-3.4
(will be tagged -3.4.1 once final)

Previously for 3.4.0 (tagged 'powerpc-darwin8-rel-3.4.0' on my github):
release-3.4.0 test logs:
http://www.csl.cornell.edu/~fang/sw/llvm/logs/llvm-release-3.4-RC1-O2-powerpc-darwin8-g++-4.0.1-debug-check.log.bz2
http://www.csl.cornell.edu/~fang/sw/llvm/logs/clang-release-3.4-RC1-O2-powerpc-darwin8-g++-4.0.1-debug-check.log.bz2

trees for 3.4.0:
https://github.com/fangism/llvm/tree/powerpc-darwin8-rel-3.4.0
https://github.com/fangism/clang/tree/powerpc-darwin8-rel-3.4.0

David

Hi Tom,

Looking at your code changes, I don’t see any issue, and 198940 should only affect the behavior of c++ program, while MultiSource/Applications/sgefa is a pure c program.

I tried top of release 3.4.1 with MultiSource/Applications/sgefa, and I can’t reproduce the issue. My dump shows with/without the patch you reverted, the test can always pass for AArch64 target.

So can you tell me on which target it failed? And can you show me the failure log as well?

Thanks,
-Jiangning

Hi Tom,

Looking at your code changes, I don't see any issue, and 198940 should only
affect the behavior of c++ program, while MultiSource/Applications/sgefa is
a pure c program.

I tried top of release 3.4.1 with MultiSource/Applications/sgefa, and I
can't reproduce the issue. My dump shows with/without the patch you
reverted, the test can always pass for AArch64 target.

So can you tell me on which target it failed? And can you show me the
failure log as well?

Attached is the failure log. The failure has been reported on x86_64
by several different testers.

-Tom

sgefa-failure.out (9.35 KB)

Hi Tom,

Now I can reproduce the issue, and sorry for any inconvenience caused!

I just realized, for branch 3.4, integrated assemble was turned on for both AArch64 and X86/X86_64. I originally thought it was not, so this is why I thought we can simply remove method Generic_ELF::IsIntegratedAssemblerDefault(). I was wrong, and we shouldn’t remove it.

With the commit 198940 applied to branch 3.4.1, the behavior of bare metal was changed, because method Target.isOSBinFormatELF() would reply true for bare metal and it checks OSes only rather than checking if format is ELF, and the bare metal would fall back to toolchains::Generic_ELF. Without this commit, originally bare metal should go to class toolchains::Generic_GCC.

Generic_GCC is the parent class of Generic_ELF, and the only difference between them is Generic_ELF enabled integrated assembler for AArch64 and X86/X86_64. With the original commit 198940, we would enable integrated assembler for bare metal, and two regression tests would fail on checking -no-integrated-as for x86 with unknown OS (bare metal).

The patch for commit 198940 intends to cover both elf format and bare metal, because there is a newly added RUN for target aarch64-none-none-eabi (bare metal). The relationship of Linux, Generic_ELF and Generic_GCC are,

Linux → Generic_ELF → Generic_GCC (-> points to parent class)

The patch 198940 originally tries to promote Linux::addClangTargetOptions to Generic_ELF::addClangTargetOptions. Considering current situation, the reasonable solution would be promoting the method Linux::addClangTargetOptions to even higher class, i.e. Generic_GCC::addClangTargetOptions. This way we could meet all requirements,

  1. Don’t change integrate assembler behavior for bare metal
  2. Enable the fix around -fuse-init-array for both ELF and bare metal

With this new promotion, the change for Driver::getToolChain in Driver.cpp would be unnecessary.

At the moment of committing 198940 on trunk, we didn’t really have this issue, because Generic_ELF implemented nothing at that moment, and integrated assembler wasn’t really turned on yet.

For top of trunk right now, I think we needn’t to promote Generic_ELF::addClangTargetOptions to Generic_GCC::addClangTargetOptions, because

  1. The implementation of isOSBinFormatELF is changed by checking ELF format directly rather than OS.
  2. Integrated assembler switch has been promoted to Generic_GCC::IsIntegratedAssemblerDefault.

Refer to attached patch, please! I tested for regression test and the LNT test MultiSource/Applications/sgefa, and I don’t see any failures. Hopefully we don’t have regressions any longer.

Thanks,
-Jiangning

198940_backport_to_release_34_clang.patch (3.55 KB)