[RFC] New function attributes for errno-setting functions

Project Infrastructure LLVM Dev List Archives
1

Hello,

Our current handling of -fno-math-errno (and this -ffast-math) in Clang is broken on systems for which libm functions do actually set errno. This is because, when -fno-math-errno is in effect, libm functions like sqrt, sin, cos, etc. are marked as readnone. As a result, these calls can be reordered with respect to other calls that set errno, and can clobber errno before it can be read. For example:

int fd = open(...);
if (fd == -1) {
  perror("oops");
}
double f = sqrt(a);

If we're on a system (like Linux, for example), where sqrt might set errno to EDOM when a is negative, we can have a problem if some optimization rearranges the code to something like this:

int fd = open(...);
double f = sqrt(a);
if (fd == -1) {
  perror("oops");
}

if the open fails, and a is -2.0, then perror will print the wrong error string (thus confusing the user). This is not really a problem with the optimization because sqrt was marked as readnone. On the other hand, we don't want to tag sqrt and writing to some arbitrary external state variable, because this will prevent autovectorization, will prevent CSE from collecting duplicate calls to sqrt, and a host of other important optimizations.

To fix this problem, I think that we need to stop treating errno as some arbitrary external state, and model is explicitly. Functions need to be able carry two additional attributes:

  1. 'writes-only-errno' - An attribute to indicate that the function may set errno, and that is the only external state to which it might write. This is useful because getModRefInfo queries can return more accurate answers when comparing to pointers that we know cannot point to errno (such as alloca'd memory and functions) [is this allowed at all?].

  2. 'errno-ignored' - Set when -fno-math-errno is in effect, indicating that it is safe to assume that the user does not care about any value of errno set by this function. This will allow CSE and autovectorization to happen (by modifying those passes to specifically query this attribute).

With these new attributes, the functions are, on systems for which libm functions might set errno, not marked readnone or readonly. This will prevent unwanted reordering. Thoughts?

Thanks again,
Hal

2

In such case it would make sense to know when "errno" is read. This way we could detect whether it's actually used, whether or not the -fno-math-errno (or some generic -fno-xyz-errno) was specified. Problem is that errno (referenced explicitly) may be a macro that expands to some system-specific function call.

On that note, what would be really nice is if a function could have an "mod/use" sets attached to it, which would specify what external symbols this function can reference. I'm not sure how that could be expressed in the current IR though.

-K

3

>
> To fix this problem, I think that we need to stop treating errno as
> some arbitrary external state, and model is explicitly.

In such case it would make sense to know when "errno" is read. This
way
we could detect whether it's actually used, whether or not the
-fno-math-errno (or some generic -fno-xyz-errno) was specified.
Problem
is that errno (referenced explicitly) may be a macro that expands to
some system-specific function call.

I agree, and this is exactly the problem: It is really hard, as far as I understand, to figure out what 'errno' actually is. On the other hand, the frontend has more information on this, at least in theory, and maybe it could communicate it to the backend somehow. Maybe the easiest way would be to insert an intrinsic @llvm.errno.read() whenever errno (as a source token) appears in the source as an rvalue (and do some similar thing when it appears as a lvalue). Thoughts?

On that note, what would be really nice is if a function could have
an
"mod/use" sets attached to it, which would specify what external
symbols
this function can reference. I'm not sure how that could be
expressed
in the current IR though.

We could attach this as metadata currently. In terms of extending the IR, we might be able to add 'implicit' function parameters similar to what we do for MIs.

-Hal

4

I think the major problem is still with "errno" defined as a preprocessor macro, as it may not look like errno by the time the front-end sees it. Perhaps some configuration test could preprocess something like "var = errno;" and check what's really getting stored in "var", but it may be unreliable (i.e. not detect all cases).

I thought a bit more about it and I think we will need to have this information to prevent similar problems as the one you mentioned in the first post. For example:

   int fd = open(...);
   int saved_errno = errno;
   double s = sqrt(var);
   if (fd == -1)
     fprintf(stderr, "error: %d\n", saved_errno);

With only the information about modifying errno, this code could be transformed to

   int fd = open(...);
   double s = sqrt(var);
   int saved_errno = errno;
   if (fd == -1)
     fprintf(stderr, "error: %d\n", saved_errno);

again producing the same situation.

-K

5

>
> Maybe the easiest way would be to insert an intrinsic
> @llvm.errno.read() whenever errno (as a source token) appears in
> the source as an rvalue (and do some similar thing when it appears
> as a lvalue). Thoughts?

I think the major problem is still with "errno" defined as a
preprocessor macro, as it may not look like errno by the time the
front-end sees it.

That's why I said source token (I meant preprocessor token), but any such solution would not be robust to preprocessed source files, and that seems like a big problem.

Perhaps some configuration test could preprocess
something like "var = errno;" and check what's really getting stored
in
"var", but it may be unreliable (i.e. not detect all cases).

I thought a bit more about it and I think we will need to have this
information to prevent similar problems as the one you mentioned in
the
first post. For example:

   int fd = open(...);
   int saved_errno = errno;
   double s = sqrt(var);
   if (fd == -1)
     fprintf(stderr, "error: %d\n", saved_errno);

With only the information about modifying errno, this code could be
transformed to

   int fd = open(...);
   double s = sqrt(var);
   int saved_errno = errno;
   if (fd == -1)
     fprintf(stderr, "error: %d\n", saved_errno);

again producing the same situation.

No, I don't think so. If sqrt is not readnone or readonly, then it should potentially alias whatever the errno macro actually expands to (whether it is a global variable, and thread-local variable, a function call, or whatever).

-Hal

6

Food for thought: If you have a codebase which can’t use -fno-math-errno, because it relies on errno values from math functions, it may be a more effective long-term strategy to work on modernizing your codebase, eliminating the dependencies on errno, rather than going through the trouble of adding even more complexity to compilers to keep errno support limping along.

Of course, whether this actually makes sense for you in your situation depends on many factors.

Dan

7

Food for thought: If you have a codebase which can't use
-fno-math-errno, because it relies on errno values from math
functions, it may be a more effective long-term strategy to work on
modernizing your codebase, eliminating the dependencies on errno,
rather than going through the trouble of adding even more complexity
to compilers to keep errno support limping along.

Wouldn't that require 'modernizing' POSIX? :wink: How else can you get the error code from open() or any number of other functions?

-Hal

8

I'm not sure about this. How about this:

extern int BLAH;
#define errno BLAH

Then we have

   int fd = open(...);
   int saved_errno = BLAH;
   double s = sqrt(var);

If "sqrt" is "write-only-errno", how would we know that BLAH should be aliased with sqrt? We don't to alias it with every global variable (that's what this proposal is trying to address). We'd need a way to associate BLAH with errno, or else we're in the same place as we are now.

-K

9

>
> No, I don't think so. If sqrt is not readnone or readonly, then it
> should potentially alias whatever the errno macro actually expands
> to (whether it is a global variable, and thread-local variable, a
> function call, or whatever).

I'm not sure about this. How about this:

extern int BLAH;
#define errno BLAH

Then we have

   int fd = open(...);
   int saved_errno = BLAH;
   double s = sqrt(var);

If "sqrt" is "write-only-errno", how would we know that BLAH should
be
aliased with sqrt? We don't to alias it with every global variable
(that's what this proposal is trying to address). We'd need a way to
associate BLAH with errno, or else we're in the same place as we are
now.

No, this is exactly what we do. It needs to be pessimistic in this sense. "write-only-errno" only helps to prove things about potential alising with pointers that we *know* can't be errno (like local stack allocations). Any global might be errno, so that can't count.

-Hal

10

open is not a math function.

Dan

11

It was pointed out to me that I misunderstood what problem you were trying to solve. Sorry for the noise.

Dan

12

>
> Food for thought: If you have a codebase which can't use
> -fno-math-errno, because it relies on errno values from math
> functions, it may be a more effective long-term strategy to work on
> modernizing your codebase, eliminating the dependencies on errno,
> rather than going through the trouble of adding even more
> complexity
> to compilers to keep errno support limping along.
>

Wouldn't that require 'modernizing' POSIX? :wink: How else can you get
the error code from open() or any number of other functions

open is not a math function.

Sorry, I misread your initial statement... but this whole issue is not about codes that need errno from math functions; that's supported just fine now. The problem is that the math functions, whether you use the errno return or not, may write to errno. And, because we currently mark these functions as readnone when -fmath-errno=0, we might reorder these functions w.r.t. other functions that do set errno, like open, and from which we care about errno's value.

-Hal

13

It was pointed out to me that I misunderstood what problem you were
trying to solve. Sorry for the noise.

No problem. FWIW, I've seen a grand total of zero production science codes that checked errno after calling a math function :wink:

-Hal

14

Do we really need the first one? We already know which standard functions could potentially set errno, and we know which standard functions don't modify any other storage (other than that which is passed via parameters). I think it's safe to assume that standard functions can be recognized by name, since the standard prohibits user objects to use a predefined name.

We could have an attribute like "side-effects-ignored" indicating that whatever modifications the given function makes, nobody will read them (at least not without an intervening store). This would include errno, but could also be used in more general contexts.

-K

15

>
> 1. 'writes-only-errno'
> 2. 'errno-ignored'

Do we really need the first one? We already know which standard
functions could potentially set errno, and we know which standard
functions don't modify any other storage (other than that which is
passed via parameters). I think it's safe to assume that standard
functions can be recognized by name, since the standard prohibits
user
objects to use a predefined name.

I agree that we could put a list into LLVM instead of adding attributes in the frontend. We need to be a little careful that -fno-builtin is passed to the backend in this case.

We could have an attribute like "side-effects-ignored" indicating
that
whatever modifications the given function makes, nobody will read
them
(at least not without an intervening store). This would include
errno,
but could also be used in more general contexts.

That would depend on how this attribute was used; it could only be used by a pass that did actually understand what those side effects were (like the loop vectorizer understanding that, if errno is ignored, then vectorizing cos() is legal). I don't object to making the name generic, but I want to make sure that we're all on the same page regarding what it means.

Thanks,
Hal

16

In some cases the exact nature of the side effects wouldn't need to be known. For example, if the side-effects are known to be ignored, a call to such a function could be eliminated as dead code.

Also, the "errno-ignored" or equivalent attribute brings up this question:
given this code
   x = cos(y); // assume "errno-ignored"
   z = global;
is it safe to infer that "global" is not errno? In other words, could this attribute be used to refine alias information with respect to errno?

-K

17

>
> That would depend on how this attribute was used; it could only be
> used by a pass that did actually understand what those side
> effects were (like the loop vectorizer understanding that, if
> errno is ignored, then vectorizing cos() is legal). I don't object
> to making the name generic, but I want to make sure that we're all
> on the same page regarding what it means.

In some cases the exact nature of the side effects wouldn't need to
be
known. For example, if the side-effects are known to be ignored, a
call
to such a function could be eliminated as dead code.

Good point.

Also, the "errno-ignored" or equivalent attribute brings up this
question:
given this code
   x = cos(y); // assume "errno-ignored"
   z = global;
is it safe to infer that "global" is not errno?

Unfortunately, I fear that the answer is no; at least, not without some further target-specific information. In general, any global could be errno. On the other hand, if we know that we're on Darwin, or using glibc, or whatever, then we could likely say something more.

-Hal

18

It has been over 5 years since I've thought about this serious, but here are some of my thoughts from then.

There are lots of good reasons to want to model errno aggressively. If you're going to do it, a function attribute "does not touch errno" is a good thing to have. The frontend then puts this on well-known libc functions.

It makes sense to model this as an attribute (as opposed to hard coding the well-known list in the optimizer) for at least three reasons:
1. The mid-level optimizer can propagate it through the callgraph.
2. Things like -fno-math-errno to set it in a front-end sensitive way.
3. Some call sites can have it, even if the function in general cannot. For example when it is statically provable that the argument to a sqrt is positive.

The major design problem you need to solve (which you identified) is that you need to know how to recognize explicit accesses to errno. You can't break code like:

  sqrt(-1);
  printf("%d", errno);

... even though errno is a macro that expands out to target-specific code for thread local errno processing.

There are better and worse ways to handle this. The ideal mode is where we can change the system header files. In that case you can change the definition of errno. For example, the mac has sys/errno.h that has:

#define errno (*__error())

we could, in principle, change this to:

#define errno (*__builtin_errno_address())

Clang would then lower this to an llvm IR "@llvm.errno.address" intrinsic, then have the code generator lower that to the right target-specific thing (a call to __error() in this case).

I don't know how feasible this is to roll out on linux or other platforms which may or may not have sympathetic libc maintainers. Assuming they aren't sympathetic, you only have one less-great option IMO. That is that you hack Clang to pattern match a well-known AST pattern into the __builtin_errno_address builtin. For targets without a well-known pattern, it would never add the "does not touch errno" attribute.

I don't think it is defensible to try to pattern match this in the IR optimizer. It would be too fragile, and putting that level of target-specific C idiom stuff into the optimizer makes me queasy.

-Chris