[PATCH] [ARM] Keep track of previous changes to the bit pattern stored HW_FP

Renato Golin renato.golin at linaro.org
Mon Jun 22 09:33:54 PDT 2015


In http://reviews.llvm.org/D10395#191658, @rs wrote:

>   // Add the features to the compile options.
>   //
>   // FIXME: If we are completely confident that we have the right set, we only
>   // need to pass the minuses.
>   Opts->Features.clear();
>   for (llvm::StringMap<bool>::const_iterator it = Features.begin(),
>          ie = Features.end(); it != ie; ++it)
>     Opts->Features.push_back((it->second ? "+" : "-") + it->first().str());
>   if (!Target->handleTargetFeatures(Opts->Features, Diags))
>     return nullptr;
>
>  


Why are you testing for _ARM_FP if the core of what you're doing is adding +/- strings to -target-features? Why not test the target-features directly?

It doesn't really matter that your StringMap is non-deterministic, if all +stuff and -stuff appear in any order, you're good.

I'm not trying to make you reproduce the old bug if it is non-deterministic (that would be folly), I'm just saying that you should test that the expected target features are coming from the right options.

If you need to test _ARM_FP, that's a different, simpler test: just put a bunch of variations and see that all of them give what you expect.

If there are already tests that do that (both target features and __ARM_FP, then just point to them and that's fine).

cheers,
--renato


http://reviews.llvm.org/D10395

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list