[PATCH] Small refactor on VectorizerHint for deduplication

Renato Golin renato.golin at linaro.org
Tue Aug 19 07:48:46 PDT 2014


Hi Arnold,

Thanks for your review. Replies inline.


>>! In D4913#10, @aschwaighofer wrote:
> I think this would reads easier if we refer to Hint and Hints instead of HintType(s).

Fair enough, I can change that.


> We concatenate and split "llvm.loop" and "vectorize.foo". I think it would simplify the code if the Hints refer to the full string?

We do that to validate and only warn if it's an invalid "llvm.loop." metadata, so we don't even scan the others (like tbaa, etc). Adding it to the name will simplify the pattern match on the hint but would make warning a bit harder. I will move the matching on the prefix to the parseHint() to make it a bit more isolated, and easier to change in the future.


> Can we centralize the logic if we move the validation logic into a member function "validate" that switches on a hint kind? Validation and the hint are intrinsically connected.

So, I though about using an enum to validate, but that meant we'd have to construct a new Hint from a string and assign a kind just to check if it's a valid hint at all, and then add InvalidHint objects and making sure none of the objects are duplicated. It got a bit complex...

Since we don't use static initializers, and this is only built once, and the constructor is statically set on the right validation lambda for each string, I thought it'd be at least safe. I agree it's not pretty.

I'll give it a few more tries...

cheers,
--renato

http://reviews.llvm.org/D4913






More information about the llvm-commits mailing list