[llvm-commits] [llvm-gcc-4.2] r49171 - in /llvm-gcc-4.2/trunk/gcc: llvm-convert.cpp llvm-types.cpp

[Duncan Sands]

Hi Dale,

> -  if (!tree_could_throw_p(exp))
> +  if (!tree_could_throw_p(exp) || !flag_exceptions)

this bit is not needed because

bool
tree_could_throw_p (tree t)
{
  if (!flag_exceptions)
    return false;

>    // Compute whether the result needs to be zext or sext'd.
>    ParameterAttributes RAttributes = HandleArgumentExtension(ReturnType);
> +  // Make sure all functions are marked nounwind if we aren't using exceptions.
> +  if (!flag_exceptions)
> +    RAttributes |= ParamAttr::NoUnwind;
> +
>    if (RAttributes != ParamAttr::None)
>      Attrs.push_back(ParamAttrsWithIndex::get(0, RAttributes));
>  
> @@ -1112,7 +1116,9 @@
>      RAttributes |= ParamAttr::NoReturn;
>  
>    // Check for 'nounwind' function attribute.
> -  if (flags & ECF_NOTHROW)
> +  // When flag_exceptions is not set (it is set by default in C++/ObjC++), 
> +  // don't unwind anything.
> +  if ((flags & ECF_NOTHROW) || !flag_exceptions)

I think this is wrong, because it means that declarations of external functions
will always be marked nounwind.  Consider compiling the following without -fexceptions:

extern void g(bool Allowed_To_Throw);
void f(void) {
  g(false);
}

and compiling the following with -fexceptions:

void g(bool Allowed_To_Throw) {
...
  if (Allowed_To_Throw)
    throw something;
  else
    abort();
...
}

then linking the two modules together.  Here g is a function that doesn't throw exceptions
unless Allowed_To_Throw is true.  With your patch g will be declared nounwind in the first
module but not in the second.  When you link them g may be marked nounwind in the linked
module which would be wrong (I don't know if this happens or not currently, but it doesn't
matter - we shouldn't rely on this kind of linker detail for correctness).

What I think should happen is:

(1) *calls* to g from the first module should be marked nounwind.  In fact all calls in
the first module should be marked nounwind.  This is already the case thanks to tree_could_throw_p.
(2) external declarations should not be marked nounwind.
(3) functions like f with a body compiled without -fexceptions should be marked nounwind.

So I think you should undo these changes, and instead add some code to StartFunctionBody
to add the nounwind attribute if !flag_exceptions.

Best wishes,

Duncan.

PS: I was wrong to think that llvm-gcc was already marking functions nounwind when compiled
with !flag_exceptions - it was prune_eh that was adding the marking.  Nice catch!