[cfe-commits] [PATCH] Add vfpv3-d16 and fix vfp2/vfp3 handling in the Clang driver

[Evgeniy Stepanov]

Yes, I also prefer the first patch. Committed as r147943 with a test.

On Wed, Jan 11, 2012 at 2:29 PM, Chandler Carruth <chandlerc at google.com> wrote:
> Hmm, I see both Anton's point about the ugliness of this code, and your
> point about there being no good way to do this.
>
> The ideal way to manage this in Clang/LLVM land is something along the lines
> of the following.
>
> Define in some class or interface an enum representing the viable modes to
> be selected here. Then provide a parse routine which converts a string into
> the enum as follows:
>
> class ... {
>   enum MyFeatureKind {
>     MFK_Foo, // "foo"
>     MFK_Bar  // "bar"
>   };
>
>   MyFeatureKind parseFeatureString(StringRef S) {
>     return llvm::StringSwitch<MyFeatureKind>(S)
>         .Case("foo", MFK_Foo)
>         .Case("bar", MFK_Bar);
>   }
> }
>
>
> And then in the driver you have your object and write:
>
>   // ...
>   switch (obj->parseFeatureString(...)) {
>     case MFK_Foo:
>       ...
>       break;
>   }
>
>
> If there happens to be a one-to-one mapping you can use a table indexed by
> the enum, or you can directly use StringSwitch to parse the string and
> produce the single value. I think if there are multiple, the switch over the
> enum is pretty clear.
>
> That said, we have a lot of code with if statements in the driver... I'm
> actually OK with your original patch's implementation as the existing code
> was already structured that way, and it's not clear there is a good place
> for this FPU enum to live, get parsed into, etc. If you do see a place, and
> would like to refactor things into a structure such as the above, awesome,
> but I'd actually prefer you apply your first patch, and *then* do the
> refactor.
>
> However, before you apply the first patch, can you add test cases? You
> should be able to add a test case under test/Driver that ensures the proper
> flags get forwarded down the chain.