[PATCH] These builtin functions set errno. Mark them accordingly.

Hal Finkel hfinkel at anl.gov
Mon May 19 10:05:04 PDT 2014


----- Original Message -----
> From: "Chris Lattner" <clattner at apple.com>
> To: reviews+D3806+public+8377178d9ac8d89d at reviews.llvm.org
> Cc: rjmccall at gmail.com, cfe-commits at cs.uiuc.edu
> Sent: Monday, May 19, 2014 11:51:30 AM
> Subject: Re: [PATCH] These builtin functions set errno. Mark them accordingly.
> 
> 
> On May 19, 2014, at 9:26 AM, Chad Rosier <mcrosier at 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.
> 
> I don’t think this is a good solution.  The better option here is to
> tell people to use -fno-math-errno if they don’t care about libm
> functions setting errno.

But this is the chicken and egg problem: is __builtin_sqrt a libm function? The problem that we currently have, IIUC, is that the answer is *sometimes*.

 -Hal

> 
> -Chris
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory




More information about the cfe-commits mailing list