[Mlir-commits] [mlir] [MLIR][LLVMIR][DLTI] Pass to update #llvm.target's features per relevant backend (PR #154938)

[Mehdi Amini]

================
@@ -403,6 +403,20 @@ ModuleFlagAttr::verify(function_ref<InFlightDiagnostic()> emitError,
                      << key << "'";
 }
 
+FailureOr<Attribute> TargetFeaturesAttr::query(DataLayoutEntryKey key) {
+  if (auto stringKey = dyn_cast<StringAttr>(key)) {
----------------
joker-eph wrote:

> As the return failure() is already right after the (single) block of the if statement, I don't think it returns earlier this way.

I don't quite get what you mean?

> Should we be returning early even in these simple cases? I applied the patch locally, but feel that in this case it doesn't really make things better.

This is the guideline because 1) it simplifies the control flow (there is one less "branch" jumping over many lines) and 2) it reduces indentation.

```
  auto stringKey = dyn_cast<StringAttr>(key));
  if (!stringKey) return failure();
 
  if (contains(stringKey))
     return UnitAttr::get(getContext());

  if (contains((std::string("+") + stringKey.strref()).str()))
    return BoolAttr::get(getContext(), true);

  if (contains((std::string("-") + stringKey.strref()).str()))
    return BoolAttr::get(getContext(), false);
  
  return failure();
```

To me this looks like a more straighline code.

https://github.com/llvm/llvm-project/pull/154938