[PATCH] D86485: [test] Fix FullUnroll.ll

Reid "Away June-Sep" Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 16 13:41:32 PDT 2020


rnk added a comment.

Here is the sequence of what happened as I understand it:

- D71687 <https://reviews.llvm.org/D71687> is intended to fix the full loop unrolling pragma with the NPM at -O1. The accompanying clang test covers this.
- D71687 <https://reviews.llvm.org/D71687> also adds an LLVM IR test. It is an integration test: it started from unoptimized IR and ran the full -O1 NPM pipeline. The IR happens to contain `optnone`. Maybe that was unintentional, since one used to be able to run `clang foo.c -emit-llvm -o - | opt -O1 -S` and get optimized IR. Now clang applies the optnone attribute, and that no longer works.
- D85578 <https://reviews.llvm.org/D85578> made the IR test more targeted: now it only runs the full unrolling pass instead of the O1 <https://reviews.llvm.org/owners/package/1/> pipeline.

There is value in integration tests, so maybe the thing to do is to bring back the IR integration test in addition to this unit test. The unit test seems correct to me. If `optnone` is present, I would not expect the loop unroll metadata to be honored.

In D86485#2234668 <https://reviews.llvm.org/D86485#2234668>, @echristo wrote:

> The code has a pragma to turn on loop unrolling. I actually did want to check with optnone.

Is that the intended behavior? Is clang supposed to fully unroll such a loop at `-O0`? I would expect it not to. The documentation doesn't say anything:
http://clang.llvm.org/docs/LanguageExtensions.html#extensions-for-loop-hint-optimizations


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86485/new/

https://reviews.llvm.org/D86485



More information about the llvm-commits mailing list