[PATCH] D15075: No error for conflict between inputs\outputs and clobber list

Akira Hatanaka via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 5 22:40:30 PDT 2016


ahatanak added inline comments.


> TargetInfo.cpp:430
>        if (AN == Name && ARN.RegNum < Names.size())
> -        return Name;
> +        if (ReturnCanonical)
> +          return Names[ARN.RegNum];

Please use braces or a ternary operator here.

> ahatanak wrote in Targets.cpp:2718
> Can we skip the checks for all these non-alphabet characters and handle them in "default"? Would doing so be incorrect?

Sorry for not being clear. Because this will fail to find registers for constraints like "=&c", I was thinking you can just check the first alphabet after skipping all the non-alphabets (using isalpha, for example).

> Targets.cpp:2714
>    }
> +  virtual StringRef getConstraintRegister(const StringRef &Constraint,
> +    const StringRef &Expression) const {

Use "override" instead of "virtual".

> SemaStmtAsm.cpp:186
> +    if (InOutVars.find(Clobber) != InOutVars.end()){
> +      SourceLocation Loc = Clobbers[i]->getLocStart();
> +      return &Loc;

You don't want to return the address of a local variable. Just change this function to return a SourceLocation by value. You can use SourceLocation::isValid in ActOnGCCAsmStmt to determine whether it is a valid SourceLocation.

> SemaStmtAsm.cpp:190
> +  }
> +  return nullptr;
> +}

Return "SourceLocation()"

> SemaStmtAsm.cpp:602
> +    Constraints, Clobbers, NumClobbers, Context.getTargetInfo(), Context);
> +  if (ConstraintLoc) {
> +    return Diag(*ConstraintLoc,

You don't need braces here.

https://reviews.llvm.org/D15075





More information about the cfe-commits mailing list