Hey Owen, Thanks for your reply. I definitely understand your perspective. Hopefully I can change it. }:-) On Sat, Apr 6, 2013 at 1:18 AM, Owen Anderson <resistor at mac.com> wrote: ...> As I've pointed out, clang does provide such functionality as an opt-in > feature through its -fsanitize options. A hypothetical Fortran frontend > could do the same, and even make it an opt-out feature if it chose. I'm > sorry if its implementation mechanism doesn't match exactly what you want > it to be, but it's not like nobody else has thought about this problem. > They have, and they've designed and shipped a solution! > > Side note: even if the -fsanitize option introduces a branch around the > division (which I haven't verified), it's quite unlikely to cause a > significant performance regression. The branch to the error block should > be perfectly predicted on any CPU made in the last 25 years. > >Even on a Xeon Phi? Just kidding around! All kidding aside, an instruction is an instruction. We have the functionality in hardware for free; it would be a shame not to use it. I should also mention that for our team, performance is job #1. Extra instructions, however insignificant, don't go over well without solid justification. Please don't misunderstand, I'm not trying to be unreasonable about this behaviour change. I do believe that there are a few strong arguments for this behaviour that we haven't touched upon yet. I also believe that this option would be an improvement to LLVM overall. But, I also haven't heard anyone in the community voicing interest. So, if no one speaks up and would like to discuss further, I'll be happy to keep such a change in my local branch. -Cameron -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130406/c66185f3/attachment.html>
On Saturday, April 6, 2013, Cameron McInally wrote:> All kidding aside, an instruction is an instruction. We have the > functionality in hardware for free; it would be a shame not to use it. I > should also mention that for our team, performance is job #1. Extra > instructions, however insignificant, don't go over well without solid > justification. >A compromise might be to add an llvm.div.or.trap intrinsic, similar to the llvm.add.with.overflow intrinsics used to implement Clang's optional integer overflow checks, which could be emitted efficiently on x86 or other platforms where division by zero or INT_MAX/-1 traps natively. -Joe -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130406/aae52318/attachment.html>
On 4/6/2013 3:07 AM, Cameron McInally wrote:> > Please don't misunderstand, I'm not trying to be unreasonable about this > behaviour change. I do believe that there are a few strong arguments for > this behaviour that we haven't touched upon yet. I also believe that > this option would be an improvement to LLVM overall. But, I also haven't > heard anyone in the community voicing interest. So, if no one speaks up > and would like to discuss further, I'll be happy to keep such a change > in my local branch.The IBM XLC compiler has a -qcheck option, which takes arguments specifying what type of runtime checks are performed. For example, -qcheck=divzero would cause the program to abort with a SIGTRAP if a division by zero was detected (the compiler would insert a conditional trap instruction). Producing a diagnostic message is one way to address this[1], but generating some sort of an exception has its merits as well---it all depends on the specific situation. I don't think that the existence of one precludes the other. [1] I've never used fsanitize---this is based on what I've read in the clang's manual. -Krzysztof -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
On Sat, Apr 6, 2013 at 10:07 AM, Cameron McInally <cameron.mcinally at nyu.edu> wrote:> Hey Owen, > > Thanks for your reply. I definitely understand your perspective. Hopefully I > can change it. }:-) > > On Sat, Apr 6, 2013 at 1:18 AM, Owen Anderson <resistor at mac.com> wrote: > ... >> >> As I've pointed out, clang does provide such functionality as an opt-in >> feature through its -fsanitize options. A hypothetical Fortran frontend >> could do the same, and even make it an opt-out feature if it chose. I'm >> sorry if its implementation mechanism doesn't match exactly what you want it >> to be, but it's not like nobody else has thought about this problem. They >> have, and they've designed and shipped a solution! >> >> Side note: even if the -fsanitize option introduces a branch around the >> division (which I haven't verified), it's quite unlikely to cause a >> significant performance regression. The branch to the error block should be >> perfectly predicted on any CPU made in the last 25 years. > > All kidding aside, an instruction is an instruction. We have the > functionality in hardware for free; it would be a shame not to use it. I > should also mention that for our team, performance is job #1. Extra > instructions, however insignificant, don't go over well without solid > justification.If you can find a way to implement -fsanitize=undefined to use the FP trap instead of a branch when checking floating division by 0, I suspect such a patch would stand a good chance of being accepted. (Although I'm not intimately familiar with the implementation, so there could be some subtlety that would prevent it.) You might need to do this in the processor-specific backend to avoid other undefined-behavior-based optimizations—that is, recognize "if (x == 0) goto err_handler; else y/x;" and replace it with "register-pc-in-fp-handler-map(); turn-on-fp-traps(); y/x;". Jeffrey
Hey Jeffrey, Thanks for the suggestion. A few comments... On Sun, Apr 7, 2013 at 12:31 PM, Jeffrey Yasskin <jyasskin at googlers.com>wrote: ...> > If you can find a way to implement -fsanitize=undefined to use the FP > trap instead of a branch when checking floating division by 0, I > suspect such a patch would stand a good chance of being accepted. > (Although I'm not intimately familiar with the implementation, so > there could be some subtlety that would prevent it.)This would work well for the constant expression division case, since the division by zero expression would be folded away leaving only a call to trap. The performance penalty for using -fsanitize on other non-constant division expressions is still unclear to me; and concerning. Although, I've been contemplating x86-64's behaviour for this case when floating point traps are disabled. Ideally, the compiler should preserve that behaviour, which might make this software implementation messy. Especially if different processors have different implementations. The simplest solution... let the hardware behave as it should.> You might need to > do this in the processor-specific backend to avoid other > undefined-behavior-based optimizations—that is, recognize "if (x == 0) > goto err_handler; else y/x;" and replace it with > "register-pc-in-fp-handler-map(); turn-on-fp-traps(); y/x;". >I believe that the constant folder would remove the constant division by zero and conditional before the backend could have its say. We would be left with only the jump to the error handler. That may complicate things. Please correct me if I am misunderstanding. Thanks again, Cameron -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130407/7a236fd4/attachment.html>