r239549 - [CodeGen] Emit Constants for immediate inlineasm arguments.

[Richard Smith]

On Fri, Jun 12, 2015 at 11:31 AM, Ahmed Bougacha <ahmed.bougacha at gmail.com>
wrote:

> On Thu, Jun 11, 2015 at 2:17 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
> > On Thu, Jun 11, 2015 at 2:13 PM, Richard Smith <richard at metafoo.co.uk>
> > wrote:
> >>
> >> On Thu, Jun 11, 2015 at 11:19 AM, Ahmed Bougacha
> >> <ahmed.bougacha at gmail.com> wrote:
> >>>
> >>> Author: ab
> >>> Date: Thu Jun 11 13:19:34 2015
> >>> New Revision: 239549
> >>>
> >>> URL: http://llvm.org/viewvc/llvm-project?rev=239549&view=rev
> >>> Log:
> >>> [CodeGen] Emit Constants for immediate inlineasm arguments.
> >>>
> >>> For inline assembly immediate constraints, we currently always use
> >>> EmitScalarExpr, instead of directly emitting the constant. When the
> >>> overflow sanitizer is enabled, this generates overflow intrinsics
> >>> instead of constants.
> >>>
> >>> Instead, emit a constant for constraints that either require an
> >>> immediate (e.g. 'I' on X86), or only accepts constants (immediate
> >>> or symbolic; i.e., don't accept registers or memory).
> >>>
> >>> Fixes PR19763.
> >>>
> >>> Differential Revision: http://reviews.llvm.org/D10255
> >>>
> >>> Added:
> >>>     cfe/trunk/test/CodeGen/inline-asm-immediate-ubsan.c
> >>> Modified:
> >>>     cfe/trunk/lib/CodeGen/CGStmt.cpp
> >>>
> >>> Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
> >>> URL:
> >>>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=239549&r1=239548&r2=239549&view=diff
> >>>
> >>>
> ==============================================================================
> >>> --- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
> >>> +++ cfe/trunk/lib/CodeGen/CGStmt.cpp Thu Jun 11 13:19:34 2015
> >>> @@ -1750,6 +1750,16 @@ llvm::Value* CodeGenFunction::EmitAsmInp
> >>>                                           const
> >>> TargetInfo::ConstraintInfo &Info,
> >>>                                             const Expr *InputExpr,
> >>>                                             std::string
> &ConstraintStr) {
> >>> +  // If this can't be a register or memory, i.e., has to be a constant
> >>> +  // (immediate or symbolic), try to emit it as such.
> >>> +  if (!Info.allowsRegister() && !Info.allowsMemory()) {
> >>> +    llvm::APSInt Result;
> >>> +    if (InputExpr->isIntegerConstantExpr(Result, getContext()))
> >>
> >>
> >> Please use EvaluateKnownConstInt here instead. This construct will
> >> redundantly re-check whether the expression is an ICE first.
> >
> >
> > Ah, sorry, you don't know that this is constant except in the
> > requiresImmediateConstant() case. You should be calling EvaluateAsInt
> here
> > instead.
>
> Yes, I'm relying on the ICE-checking here, but I just realized Sema
> also calls EvaluateAsInt, and because of that doesn't check that the
> requiresImmediateConstant() operand is an ICE.
>
> Does the below patch make sense?  It lets us emit an error for the
> overflowing "constant" expressions we want to catch here.
>

Yes, that looks fine. Please also include a testcase (if you need an
example of a non-ICE that we can evaluate, something like "(long)(void*)0"
should work).

With that, we can use EvaluateAsInt in CGStmt.cpp, and we'll do the
> right thing for all combinations except overflowing "i".  I'm not
> quite sure how to handle that, if it's even possible in the general
> case: what happens if a link-time constant expression overflows?
>

In C++11 onwards, it's not an ICE. In any case, the evaluator will produce
the two's complement result.

Thanks for having a look!
> -Ahmed
>
>
> diff --git a/cfe/trunk/lib/Sema/SemaStmtAsm.cpp
> b/cfe/trunk/lib/Sema/SemaStmtAsm.cpp
> --- a/cfe/trunk/lib/Sema/SemaStmtAsm.cpp (revision 239545)
> +++ b/cfe/trunk/lib/Sema/SemaStmtAsm.cpp (working copy)
> @@ -255,7 +255,7 @@
>                           << InputExpr->getSourceRange());
>      } else if (Info.requiresImmediateConstant() &&
> !Info.allowsRegister()) {
>        llvm::APSInt Result;
> -      if (!InputExpr->EvaluateAsInt(Result, Context))
> +      if (!InputExpr->isIntegerConstantExpr(Result, Context))
>          return StmtError(
>              Diag(InputExpr->getLocStart(),
> diag::err_asm_immediate_expected)
>              << Info.getConstraintStr() << InputExpr->getSourceRange());
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150612/c7896a33/attachment.html>