r251371 - Use early exits to reduce indentation.

Eric Christopher via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 26 23:20:54 PDT 2015


Thank you very much for the suggestion, it helped clean up the algorithm
for what I was trying to do in r251388.

-eric

On Mon, Oct 26, 2015 at 5:12 PM Eric Christopher <echristo at gmail.com> wrote:

> On Mon, Oct 26, 2015 at 5:11 PM David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Mon, Oct 26, 2015 at 5:06 PM, Eric Christopher via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: echristo
>>> Date: Mon Oct 26 19:06:21 2015
>>> New Revision: 251371
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=251371&view=rev
>>> Log:
>>> Use early exits to reduce indentation.
>>>
>>> Modified:
>>>     cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>>>
>>> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=251371&r1=251370&r2=251371&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Mon Oct 26 19:06:21 2015
>>> @@ -296,53 +296,55 @@ bool CodeGenFunction::checkBuiltinTarget
>>>    if (!TargetDecl)
>>>      return true;
>>>
>>> -  // Get the current enclosing function if it exists.
>>> -  if (const FunctionDecl *FD =
>>> dyn_cast_or_null<FunctionDecl>(CurFuncDecl)) {
>>> -    unsigned BuiltinID = TargetDecl->getBuiltinID();
>>> -    const char *FeatureList =
>>> -        CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
>>> -    if (FeatureList && StringRef(FeatureList) != "") {
>>> -      StringRef TargetCPU = Target.getTargetOpts().CPU;
>>> -      llvm::StringMap<bool> FeatureMap;
>>> -
>>> -      if (const auto *TD = FD->getAttr<TargetAttr>()) {
>>> -        // If we have a TargetAttr build up the feature map based on
>>> that.
>>> -        TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
>>> -
>>> -        // Make a copy of the features as passed on the command line
>>> into the
>>> -        // beginning of the additional features from the function to
>>> override.
>>> -        ParsedAttr.first.insert(
>>> -            ParsedAttr.first.begin(),
>>> -            Target.getTargetOpts().FeaturesAsWritten.begin(),
>>> -            Target.getTargetOpts().FeaturesAsWritten.end());
>>> -
>>> -        if (ParsedAttr.second != "")
>>> -          TargetCPU = ParsedAttr.second;
>>> -
>>> -        // Now populate the feature map, first with the TargetCPU which
>>> is
>>> -        // either
>>> -        // the default or a new one from the target attribute string.
>>> Then we'll
>>> -        // use the passed in features (FeaturesAsWritten) along with
>>> the new
>>> -        // ones
>>> -        // from the attribute.
>>> -        Target.initFeatureMap(FeatureMap, CGM.getDiags(), TargetCPU,
>>> -                              ParsedAttr.first);
>>> -      } else {
>>> -        Target.initFeatureMap(FeatureMap, CGM.getDiags(), TargetCPU,
>>> -                              Target.getTargetOpts().Features);
>>> -      }
>>> -
>>> -      // If we have at least one of the features in the feature list
>>> return
>>> -      // true, otherwise return false.
>>> -      SmallVector<StringRef, 1> AttrFeatures;
>>> -      StringRef(FeatureList).split(AttrFeatures, ",");
>>> -      for (const auto &Feature : AttrFeatures)
>>> -        if (FeatureMap[Feature])
>>> -         return true;
>>> -      return false;
>>> -    }
>>> +  // Get the current enclosing function if it exists. If it doesn't
>>> +  // we can't check the target features anyhow.
>>> +  const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CurFuncDecl);
>>> +  if (!FD) return true;
>>> +
>>> +  unsigned BuiltinID = TargetDecl->getBuiltinID();
>>> +  const char *FeatureList =
>>> +      CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
>>> +
>>> +  if (!FeatureList || StringRef(FeatureList) == "")
>>> +    return true;
>>> +
>>> +  StringRef TargetCPU = Target.getTargetOpts().CPU;
>>> +  llvm::StringMap<bool> FeatureMap;
>>> +
>>> +  if (const auto *TD = FD->getAttr<TargetAttr>()) {
>>> +    // If we have a TargetAttr build up the feature map based on that.
>>> +    TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
>>> +
>>> +    // Make a copy of the features as passed on the command line into
>>> the
>>> +    // beginning of the additional features from the function to
>>> override.
>>> +    ParsedAttr.first.insert(ParsedAttr.first.begin(),
>>> +
>>> Target.getTargetOpts().FeaturesAsWritten.begin(),
>>> +
>>> Target.getTargetOpts().FeaturesAsWritten.end());
>>> +
>>> +    if (ParsedAttr.second != "")
>>> +      TargetCPU = ParsedAttr.second;
>>> +
>>> +    // Now populate the feature map, first with the TargetCPU which is
>>> +    // either
>>> +    // the default or a new one from the target attribute string. Then
>>> we'll
>>> +    // use the passed in features (FeaturesAsWritten) along with the new
>>> +    // ones
>>> +    // from the attribute.
>>> +    Target.initFeatureMap(FeatureMap, CGM.getDiags(), TargetCPU,
>>> +                          ParsedAttr.first);
>>> +  } else {
>>> +    Target.initFeatureMap(FeatureMap, CGM.getDiags(), TargetCPU,
>>> +                          Target.getTargetOpts().Features);
>>>    }
>>> -  return true;
>>> +
>>> +  // If we have at least one of the features in the feature list return
>>> +  // true, otherwise return false.
>>> +  SmallVector<StringRef, 1> AttrFeatures;
>>> +  StringRef(FeatureList).split(AttrFeatures, ",");
>>> +  for (const auto &Feature : AttrFeatures)
>>> +    if (FeatureMap[Feature])
>>> +      return true;
>>> +  return false;
>>>
>>
>> This last bit could be written as:
>>
>> return std::any_of(AttrFeatures.begin(), AttrFeatures.end(), [&](mumble
>> &Feature) { return FeatureMap[Feature]; });
>>
>> if you like/if that seems better.
>>
>> (also, I wouldn't mind if people start writing llvm versions of the
>> standard algorithms that take a range instead of a begin/end iterator pair
>> either... )
>>
>
> That would totally be awesome, except I'm about to refactor that code more.
>
> But I'll keep it in mind and see if I can manage it :)
>
> -eric
>
>
>>
>>
>>>  }
>>>
>>>  RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151027/1b48b082/attachment-0001.html>


More information about the cfe-commits mailing list