Removing metadata in a pass

Is it OK to remove metadata in an optimization pass? The context is patch http://reviews.llvm.org/D4571 which removes loop unrolling hint metadata after using it to avoid unrolling more than the hint suggests. This is a potential problem because loop unrolling can be run more than once. Example: a loop annotated with “#pragma clang loop unroll_count(2)” which adds hint metadata to the loop would be unrolled twice every time the loop unrolling pass is run. Anyway, I ask about metadata removal because Eli who is reviewing the patch wasn’t sure whether this was acceptable.

Loop unrolling metadata can take the following forms:
llvm.loop.unroll.enable false // don’t unroll
llvm.loop.unroll.enable true // try to fully unroll
llvm.loop.unroll.count N // try to unroll N times

An alternative implementation to deleting nodes would be to just add a “llvm.loop.unroll.enable false” metadata node after unrolling. It’s a little funny because you could then have, say, both “llvm.loop.unroll.enable false” and “llvm.loop.unroll.enable true” attached to a loop which is a bit funny. Yet another alternative is to make up some new metadata node type like “llvm.loop.unroll.already_unrolled” and add it to the loop.

Mark

In general metadata is designed so it can be safely dropped without
changing behavior of the resulting code. It can, however have an
effect on the optimizer. IMO dropping it is also the right thing to do
in your case as you fulfilled the request to unroll the loop.

- Ben

An alternative implementation to deleting nodes would be to just add a
"llvm.loop.unroll.enable false" metadata node after unrolling. It's a
little funny because you could then have, say, both "llvm.loop.unroll.enable
false" and "llvm.loop.unroll.enable true" attached to a loop which is a bit
funny. Yet another alternative is to make up some new metadata node type
like "llvm.loop.unroll.already_unrolled" and add it to the loop.

What about dividing the unroll hint by the unroll count and dropping
it if that's <= 1?

Cheers.

Tim.

This is a destructive change very similar to Mark's original in the patch.
My issue with destructive changes is that they may make things more
difficult to debug by "deleting the audit trail", so to say. But this isn't
a strong objection, so if others feel it's appropriate in this case, I'm
fine with that.

Eli

What about dividing the unroll hint by the unroll count and dropping
it if that's <= 1?

That would usually get to an unroll factor closer to what the user
requested, though most of the time they wouldn't be divisible so you'd
rarely end up with the exact factor in the end. I'm a little on the fence
about the compiler doing any unrolling if the pragma target can't be met.
Maybe it should just emit a warning and give up. Currently it unrolls as
much as possible if the target can't be met (it will emit a warning in this
case once I finish a patch I'm working on). Though I can see bike-shedding
arguments for either case.

Mark

From: "Mark Heffernan" <meheff@google.com>
To: "Tim Northover" <t.p.northover@gmail.com>
Cc: "LLVM Developers Mailing List" <llvmdev@cs.uiuc.edu>
Sent: Friday, July 18, 2014 11:41:56 AM
Subject: Re: [LLVMdev] Removing metadata in a pass

What about dividing the unroll hint by the unroll count and dropping
it if that's <= 1?

That would usually get to an unroll factor closer to what the user
requested, though most of the time they wouldn't be divisible so
you'd rarely end up with the exact factor in the end. I'm a little
on the fence about the compiler doing any unrolling if the pragma
target can't be met. Maybe it should just emit a warning and give
up. Currently it unrolls as much as possible if the target can't be
met (it will emit a warning in this case once I finish a patch I'm
working on). Though I can see bike-shedding arguments for either
case.

I agree; I think that they'll rarely be divisible. Also, I agree that a warning would be nice.

Since you started by asking a general question, let me answer that: A pass is *required* to remove all metadata on any instructions that it changes and/or moves that it does not know will still be valid after the transformation. Now optimization hints don't necessarily have validity constraints, and so the answer is fuzzy here, but some metadata does, and this is something to keep in mind.

-Hal