----- Original Message -----
> From: "Chandler Carruth" <chandl...@gmail.com>
> To: reviews+d3806+public+8377178d9ac8d...@reviews.llvm.org
> Cc: "John McCall" <rjmcc...@gmail.com>, "llvm cfe" <cfe-commits@cs.uiuc.edu>
> Sent: Monday, May 19, 2014 12:35:47 PM
> Subject: Re: [PATCH] These builtin functions set errno. Mark them accordingly.
>
>
>
>
>
>
> On Mon, May 19, 2014 at 10:26 AM, Chad Rosier <
> mcros...@codeaurora.org > wrote:
>
>
>
> Hal and I discussed this on IRC and here's the synopsis (mostly in
> Hal's words):
>
> Many of these builtins turn into libc calls, and those do set errno.
> Marking them as readnone is a problem, because we can
> reorder/eliminate writes to errno. For example, if we assume
> readnone for something like __builtin_sqrt(); write(); perror(); it
> may be reordered to write(); __builtin_sqrt(); perror(); and perror
> may provide junk. However, if we don't mark the builtins as
> readnone, then they won't get lowered into ISD nodes (for those
> nodes that are relevant), and that means that they won't get
> instruction selected into native instructions on targets where
> that's possible. So the __builtin_sqrt is kind of an escape hatch to
> force the direct lowering behavior even when we can't do it for sqrt
> because sqrt might set errno. But, as Hal said, this behavior *is*
> broken, technically. And, in fact, it is not hard to create test
> cases where you can see buggy behavior. Hak's just not sure what the
> right way of handling it is. Hal thinks we really need some kind of
> target inp!
> ut, so that we can know ahead of time whether the builtin will
> *really* set errno or not after isel. Hal thinks there are two
> solutions possible.
>
> 1. Like with inline asm registers/constraints; each target will
> provide a table describing what builtins will really not set errno.
> 2. Let Clang essentially query TTI for those builtins that are
> relevant. This second method will require a bit of infrastructure
> work, but is probably not too difficult.
>
> I'm not going to push this patch further because it's not the right
> solution. In theory, I believe it is correct, but it breaks the
> __buitlin escape hatch, which would likely result in serious
> performance degradations.
> Crazy idea:
>
>
> When we lower one of these to a call to a libc function, save and
> restore errno around it.
Actually, I really like this idea. It will take some work, however, because
errno is not a particular symbol (it is almost always a macro that expands to
something else), and it changes depending on what OS/libc is being used. Clang
has, at least in theory, all of the information necessary to generate a
save/restore function for errno (although it may need to force the inclusion of
errno.h).
For example, we could do something like this:
1. In clang::InitializePreprocessor, add something like:
Builder.append("#include <errno.h>
static int __read_errno() { return errno; }
__attribute__((always_inline))
static void __write_errno(int e) { errno = e; }
__attribute__((always_inline))");
2. When we lower these __builtin_<foo> functions, add code around them using
__read_errno and __write_errno so that errno is not
clobbered even if the builtin is lowered to a libm call.
3. Make sure that DAGCombine and/or DeadMachineInstructionElim removes the
save/restore code when it is dead (because the builtin was lowered to an
instruction) for common platforms.
-Hal
>
>
> Too crazy?
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
