Ariel Ben-Yehuda via llvm-dev
2017-May-29 21:41 UTC
[llvm-dev] The state of ARMConstantIslandPass in 4.0.[01]
Hi, We at Rust are using LLVM for our codegen. Since Rust version 1.19.0, which should ship in July 21st, we are using a slightly patched version of LLVM 4.0. The transition to LLVM 4.0 had been fairly pain-free on x86 (at least excluding the standard suite of assertion failures using MSVC SEH, but that's fairly under control), but we have encountered several scary bugs in ARM, most specifically in ARMConstantIslandPass.cpp: - https://github.com/rust-lang/rust/issues/41672 - https://github.com/rust-lang/rust/issues/42248 The former of these is a fairly benign assertion failure, but the latter is a scary "in the wild" wrong codegen bug, which can really mess up with programmers. Both of the bugs have fixes in trunk: r295964 - Fix assertion failure in ARMConstantIslandPass. r297871 - ARM: avoid clobbering register in v6 jump-table expansion. I can easily backport the fixes for these 2 bugs into our LLVM branch. However, while looking at the diff between 4.0 and trunk I've seen several more fixes to "scary-looking" bugs: r294949 - [Thumb-1] TBB generation: spot redefinitions of index register r295816 - [ARM] Fix constant islands pass. r299634 - [ARM] Remove a dead ADD during the creation of TBBs (that one looks more like an optimization than a wrong-codegen thing, but including for completeness) r300870 - [Thumb-1] Fix corner cases for compressed jump tables After seeing the previous ones, I'm worried these other fixes might also fix some "in the wild" wrong codegen bugs. However, I'm not comfortable backporting so many patches by myself. We are open to moving to being based on LLVM 4.0.1 when it is released, but it does not seem to include *any* of the above fixes. Are these fixes important? Should we backport them? Do we need to keep anything in mind? Thanks in advance, - Ariel
Tom Stellard via llvm-dev
2017-May-29 22:27 UTC
[llvm-dev] The state of ARMConstantIslandPass in 4.0.[01]
On 05/29/2017 05:41 PM, Ariel Ben-Yehuda via llvm-dev wrote:> Hi, > > We at Rust are using LLVM for our codegen. Since Rust version 1.19.0, > which should ship in July 21st, we are using a slightly patched > version of LLVM 4.0. > > The transition to LLVM 4.0 had been fairly pain-free on x86 (at least > excluding the standard suite of assertion failures using MSVC SEH, but > that's fairly under control), but we have encountered several scary > bugs in ARM, most specifically in ARMConstantIslandPass.cpp: > > - https://github.com/rust-lang/rust/issues/41672 > - https://github.com/rust-lang/rust/issues/42248 > > The former of these is a fairly benign assertion failure, but the > latter is a scary "in the wild" wrong codegen bug, which can really > mess up with programmers. > > Both of the bugs have fixes in trunk: > r295964 - Fix assertion failure in ARMConstantIslandPass. > r297871 - ARM: avoid clobbering register in v6 jump-table expansion. > > I can easily backport the fixes for these 2 bugs into our LLVM branch. > However, while looking at the diff between 4.0 and trunk I've seen > several more fixes to "scary-looking" bugs: > > r294949 - [Thumb-1] TBB generation: spot redefinitions of index register > r295816 - [ARM] Fix constant islands pass. > r299634 - [ARM] Remove a dead ADD during the creation of TBBs > (that one looks more like an optimization than a wrong-codegen > thing, but including for completeness) > r300870 - [Thumb-1] Fix corner cases for compressed jump tables > > After seeing the previous ones, I'm worried these other fixes might > also fix some "in the wild" wrong codegen bugs. However, I'm not > comfortable backporting so many patches by myself. We are open to > moving to being based on LLVM 4.0.1 when it is released, but it does > not seem to include *any* of the above fixes. > > Are these fixes important? Should we backport them? Do we need to keep > anything in mind? >It's probably too late for these to make it into the 4.0.1 release, since the merge deadline is today, but please submit merge requests for these anyway (preferred way is to use the utils/release/merge-request.sh script), because it will help us determine if we need a 4.0.2 release. Thanks, Tom> Thanks in advance, > - Ariel > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >
Ariel Ben-Yehuda via llvm-dev
2017-May-29 22:44 UTC
[llvm-dev] The state of ARMConstantIslandPass in 4.0.[01]
Opened merge requests: https://bugs.llvm.org/show_bug.cgi?id=33213 https://bugs.llvm.org/show_bug.cgi?id=33214 https://bugs.llvm.org/show_bug.cgi?id=33215 https://bugs.llvm.org/show_bug.cgi?id=33216 https://bugs.llvm.org/show_bug.cgi?id=33217 https://bugs.llvm.org/show_bug.cgi?id=33218 On Tue, May 30, 2017 at 1:27 AM, Tom Stellard <tstellar at redhat.com> wrote:> On 05/29/2017 05:41 PM, Ariel Ben-Yehuda via llvm-dev wrote: >> Hi, >> >> We at Rust are using LLVM for our codegen. Since Rust version 1.19.0, >> which should ship in July 21st, we are using a slightly patched >> version of LLVM 4.0. >> >> The transition to LLVM 4.0 had been fairly pain-free on x86 (at least >> excluding the standard suite of assertion failures using MSVC SEH, but >> that's fairly under control), but we have encountered several scary >> bugs in ARM, most specifically in ARMConstantIslandPass.cpp: >> >> - https://github.com/rust-lang/rust/issues/41672 >> - https://github.com/rust-lang/rust/issues/42248 >> >> The former of these is a fairly benign assertion failure, but the >> latter is a scary "in the wild" wrong codegen bug, which can really >> mess up with programmers. >> >> Both of the bugs have fixes in trunk: >> r295964 - Fix assertion failure in ARMConstantIslandPass. >> r297871 - ARM: avoid clobbering register in v6 jump-table expansion. >> >> I can easily backport the fixes for these 2 bugs into our LLVM branch. >> However, while looking at the diff between 4.0 and trunk I've seen >> several more fixes to "scary-looking" bugs: >> >> r294949 - [Thumb-1] TBB generation: spot redefinitions of index register >> r295816 - [ARM] Fix constant islands pass. >> r299634 - [ARM] Remove a dead ADD during the creation of TBBs >> (that one looks more like an optimization than a wrong-codegen >> thing, but including for completeness) >> r300870 - [Thumb-1] Fix corner cases for compressed jump tables >> >> After seeing the previous ones, I'm worried these other fixes might >> also fix some "in the wild" wrong codegen bugs. However, I'm not >> comfortable backporting so many patches by myself. We are open to >> moving to being based on LLVM 4.0.1 when it is released, but it does >> not seem to include *any* of the above fixes. >> >> Are these fixes important? Should we backport them? Do we need to keep >> anything in mind? >> > > It's probably too late for these to make it into the 4.0.1 release, > since the merge deadline is today, but please submit merge requests > for these anyway (preferred way is to use the utils/release/merge-request.sh > script), because it will help us determine if we need a 4.0.2 release. > > Thanks, > Tom > >> Thanks in advance, >> - Ariel >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >
Renato Golin via llvm-dev
2017-May-30 07:34 UTC
[llvm-dev] The state of ARMConstantIslandPass in 4.0.[01]
On 29 May 2017 at 22:41, Ariel Ben-Yehuda via llvm-dev <llvm-dev at lists.llvm.org> wrote:> Are these fixes important? Should we backport them? Do we need to keep > anything in mind?Hi Ariel, Those fixes are important, but as Tom said, only for 4.0.2 if we do one. However, they'll need to be looked at with care, as they fix some odd bugs in the compiler and I'm not sure how stable they really are. They may behave well in trunk because of other patches that were noticed and fixed, but back-porting just those patches may introduce instabilities that weren't fixed in 4.0. Given that we release a new version every 6 months, and that we (Still) don't have (community) support for older versions, you may be better off using 5.0 instead. Thanks for creating the merge requests, though. Let's look at them when/if 4.0.2 comes along. cheers, --renato
Ariel Ben-Yehuda via llvm-dev
2017-May-30 09:06 UTC
[llvm-dev] The state of ARMConstantIslandPass in 4.0.[01]
On 5/30/17, Renato Golin <renato.golin at linaro.org> wrote:> On 29 May 2017 at 22:41, Ariel Ben-Yehuda via llvm-dev > <llvm-dev at lists.llvm.org> wrote: >> Are these fixes important? Should we backport them? Do we need to keep >> anything in mind? > > Hi Ariel, > > Those fixes are important, but as Tom said, only for 4.0.2 if we do one. > > However, they'll need to be looked at with care, as they fix some odd > bugs in the compiler and I'm not sure how stable they really are. > > They may behave well in trunk because of other patches that were > noticed and fixed, but back-porting just those patches may introduce > instabilities that weren't fixed in 4.0. > > Given that we release a new version every 6 months, and that we > (Still) don't have (community) support for older versions, you may be > better off using 5.0 instead. > > Thanks for creating the merge requests, though. Let's look at them > when/if 4.0.2 comes along. > > cheers, > --renato >> They may behave well in trunk because of other patches that werenoticed and fixed, but back-porting just those patches may introduce instabilities that weren't fixed in 4.0. That's what's worrying me. Because we don't want to ship a version of Rust on which developing on ARM is a minefield, we have to apply the first 2 patches. However, it looks that we might also need to apply the other patches, and I'm worried about whether and how can they be safely backported. Could the patch authors pitch in?> Given that we release a new version every 6 months, and that we (Still) don't have (community) support for older versions, you may be better off using 5.0 instead.In our experience, every new version of LLVM both requires some porting work and introduces new issues in non-P1 targets that need to be fixed. We'll upgrade to LLVM 5.0 as soon as it gets released and we port and apply the fixes. In the meanwhile we'll like a version in which ARM is less of a minefield. - Ariel