================

----------------
madhur13490 wrote:

> Should this verify that enable and disable are mutually exclusive?

Yes, I think so. We need two solutions here:
1. Verifier should disallow the presence of both `.enable` and `.disable` 
nodes. There is no point in having both.
2. However, verifier is not guarateed to run all the times and passes may hold 
both nodes transiently. I see that `setForced()` currently lets `.enable` win 
while `hasDistributeTransformation()` lets .disable win. We should give 
`.disable` precedence. It matches every other loop-transform reader — 
`hasUnrollTransformation()`, `hasUnrollAndJamTransformation()`, 
`hasVectorizeTransformation()`, and `hasDistributeTransformation() all check 
.disable first.

(rejecting contradictory markers is idiomatic in the verifier — 
zeroext/signext, readnone/readonly, the DI hasConflictingReferenceFlags helper, 
etc.). This would be the first loop-metadata pair to enforce it; the 
unroll/vectorize pairs only rely on reader precedence today, so we can extend 
the same check to those as a follow-up.

> I'm also confused why this is turning a parameterized metadata into 2 ... I 
> expect optimization annotations to work in one direction, and there to be no 
> distinct meaning between no annotation and a false.

For distribution, "disable" is genuinely distinct from "absent" — it's 
tri-state, not one-directional:

1. enable → force distribution on;
2. disable → force it off even when distribution is enabled globally 
3. absent → follow the global default / heuristic.
That's exactly the existing TransformationMode tri-state (TM_ForcedByUser / 
TM_SuppressedByUser / TM_Unspecified) shared by all loop transforms, so we 
can't collapse it to enable-only.

If this make sense, I'll have a fixup commit to address.


https://github.com/llvm/llvm-project/pull/201077
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to