November 30, 2017
On 11/30/2017 3:51 PM, Nicholas Wilson wrote:
> On Thursday, 30 November 2017 at 18:18:41 UTC, Jonathan M Davis wrote:
>> But I have a hard time believing that the cost of assertions relates to constructing an AssertError unless the compiler is inlining a bunch of stuff at the assertion site. If that's what's happening, then it would increase the code size around assertions and potentially affect performance.
>>
>> - Jonathan M Davis
> 
> Indeed, if DMD is not marking the conditional call to _d_assert (or whatever it is) 'cold' and the call itself `pragma(inline, false)` then it needs to be changed to do so.

Instead of speculation, let's look at what actually happens:

---------------------------------
  void test(int i) {
    assert(i, "message");
  }
---------------------------------
dmd -c -m64 -O test
obj2asm -x test.obj
---------------------------------

__a6_746573742e64:
        db      074h,065h,073h,074h,02eh,064h,000h      ;test.d.
__a7_6d657373616765:
        db      06dh,065h,073h,073h,061h,067h,065h,000h ;message.

_D4test4testFiZv:
0000:           push    RBP
0001:           mov     RBP,RSP
0004:           sub     RSP,040h
0008:           mov     010h[RBP],ECX
000b:           cmp     dword ptr 010h[RBP],0
000f:           jne     $+3Ah
--- start of inserted assert failure code ---
0011:           mov     R8D,5                     // line number
0017:           lea     RAX,FLAT:_BSS[00h][RIP]
001e:           mov     -018h[RBP],RAX            // filename.ptr
0022:           mov     qword ptr -020h[RBP],6    // filename.length
002a:           lea     RDX,-020h[RBP]            // &filename[]
002e:           lea     RCX,FLAT:_BSS[00h][RIP]
0035:           mov     -8[RBP],RCX               // msg.ptr
0039:           mov     qword ptr -010h[RBP],7    // msg.length
0041:           lea     RCX,-010h[RBP]            // &msg[]
0045:           call      L0
--- end of inserted assert failure code ---
004a:           mov     RSP,RBP
004d:           pop     RBP
004e:           ret
-------------------------------------------

26 bytes of inserted Bloaty McBloatface code and 15 bytes of data. My proposal:

_D4test4testFiZv:
0000:           push    RBP
0001:           mov     RBP,RSP
0004:           sub     RSP,040h
0008:           mov     010h[RBP],ECX
000b:           cmp     dword ptr 010h[RBP],0
000f:           jne     $+01h
0011:           hlt                                // look ma, 1 byte!
0012:           mov     RSP,RBP
0015:           pop     RBP
0016:           ret

1 byte of inserted code, and the data strings are gone as well.
December 01, 2017
On Friday, 1 December 2017 at 03:23:23 UTC, Walter Bright wrote:
> 26 bytes of inserted Bloaty McBloatface code and 15 bytes of data. My proposal:

I suggest we break up the -release switch into different options. I never, never use -release since it implies no bounds checking. But if we wanted small asserts, we'd ideally like -slimassert perhaps to change that behavior without killing arrays too.

> 0011:           hlt                                // look ma, 1 byte!

BTW, are you against using `int 3` as the opcode instead? (0xCC)

hlt kinda bothers me because it actually has a meaning. You're just abusing the fact that it is privileged so it traps on operating systems, but on bare metal, it just pauses until the next interrupt.

int 3, on the other hand, is explicitly for debugging - which is what we want asserts to be.
December 01, 2017
On Friday, 1 December 2017 at 03:43:07 UTC, Adam D. Ruppe wrote:
> On Friday, 1 December 2017 at 03:23:23 UTC, Walter Bright wrote:
>> 26 bytes of inserted Bloaty McBloatface code and 15 bytes of data. My proposal:
> [...]
> int 3, on the other hand, is explicitly for debugging - which is what we want asserts to be.

Aren't hardware breakpoints limited in numbers ?
November 30, 2017
On Friday, December 01, 2017 03:43:07 Adam D. Ruppe via Digitalmars-d wrote:
> On Friday, 1 December 2017 at 03:23:23 UTC, Walter Bright wrote:
> > 26 bytes of inserted Bloaty McBloatface code and 15 bytes of
>
> > data. My proposal:
> I suggest we break up the -release switch into different options. I never, never use -release since it implies no bounds checking. But if we wanted small asserts, we'd ideally like -slimassert perhaps to change that behavior without killing arrays too.

It only implies no bounds checking in @system code, though obviously, if you're not marking code with @safe or simply want the bounds checking to be enabled in @system code, then using -release isn't something that you're going to want to do. Regardless, I don't think that it makes sense for -release to imply any kind of enabling of assertions, since a lot of folks write assertions with the idea that they're only going to be enabled in debug builds, and sometimes, what's being asserted isn't cheap. So, we do need to either have a different flag to enable stripped down assertions in a release build or have some sort of argument to -release that controls what it does (which has been suggested for the DIP related to controlling contracts). And once you're talking about enabling some kind of assertion in a release build, that begs the question of whether contracts and invariants should be included. Pretty quickly, it sounds like what -release does isn't appropriate at all, and a debug build with optimizations enabled makes more sense - though it would still need a flag of some kind to indicate that you wanted the stripped down assertions.

- Jonathan M Davis

December 01, 2017
On Friday, 1 December 2017 at 03:52:01 UTC, user1234 wrote:
> Aren't hardware breakpoints limited in numbers ?

I've never heard that and can't think of any reason why it would be. The instruction as far as the CPU is concerned is to just trigger the behavior when it is executed, it doesn't know or care how many times it appears.
December 01, 2017
On Friday, 1 December 2017 at 04:07:24 UTC, Jonathan M Davis wrote:
> It only implies no bounds checking in @system code, though obviously, if you're not marking code with @safe or simply want the bounds checking to be enabled in @system code, then using -release isn't something that you're going to want to do.

Indeed, but disabling bounds checking in @system code is trivial anyway (just use `.ptr` in the index expression, and that's the level it should be done - individual expressions that you've verified somehow already).

> Pretty quickly, it sounds like what -release does isn't appropriate at all

Well, my real point is that -release is too blunt to ever use. Every time it is mentioned, my reply is "-release is extremely harmful, NEVER use it", since it does far too much stuff at once.

So what I'm asking for here is just independent switches to control the other stuff too. Like we used to just have `-release`, but now we have `-boundscheck=[on|safeonly|off]`.

I propose we also add `-assert=[throws|c|traps|off]` and `-contracts=[on|off]` and perhaps `-invariants=[on|off]` if they aren't lumped into contracts.

Of those assert options:

throws = what normal D does now (throw AssertError)
c = what -betterC D does now (call the C runtime assert)
traps = emit the hardware instruction (whether hlt or int 3)
off = what -release D does now (omit entirely, except the assert(0) case, which traps)


Then, we can tweak the asserts without also killing D's general memory safety victory that bounds checking brings.
December 01, 2017
On Friday, 1 December 2017 at 03:23:23 UTC, Walter Bright wrote:
> On 11/30/2017 3:51 PM, Nicholas Wilson wrote:
>> On Thursday, 30 November 2017 at 18:18:41 UTC, Jonathan M Davis wrote:
>>> But I have a hard time believing that the cost of assertions relates to constructing an AssertError unless the compiler is inlining a bunch of stuff at the assertion site. If that's what's happening, then it would increase the code size around assertions and potentially affect performance.
>>>
>>> - Jonathan M Davis
>> 
>> Indeed, if DMD is not marking the conditional call to _d_assert (or whatever it is) 'cold' and the call itself `pragma(inline, false)` then it needs to be changed to do so.
>
> Instead of speculation, let's look at what actually happens:
>
> ---------------------------------
>   void test(int i) {
>     assert(i, "message");
>   }
> ---------------------------------
> dmd -c -m64 -O test
> obj2asm -x test.obj
> ---------------------------------
>
> __a6_746573742e64:
>         db      074h,065h,073h,074h,02eh,064h,000h      ;test.d.
> __a7_6d657373616765:
>         db      06dh,065h,073h,073h,061h,067h,065h,000h ;message.
>
> _D4test4testFiZv:
> 0000:           push    RBP
> 0001:           mov     RBP,RSP
> 0004:           sub     RSP,040h
> 0008:           mov     010h[RBP],ECX
> 000b:           cmp     dword ptr 010h[RBP],0
> 000f:           jne     $+3Ah
> --- start of inserted assert failure code ---
> 0011:           mov     R8D,5                     // line number
> 0017:           lea     RAX,FLAT:_BSS[00h][RIP]
> 001e:           mov     -018h[RBP],RAX            // filename.ptr
> 0022:           mov     qword ptr -020h[RBP],6    // filename.length
> 002a:           lea     RDX,-020h[RBP]            // &filename[]
> 002e:           lea     RCX,FLAT:_BSS[00h][RIP]
> 0035:           mov     -8[RBP],RCX               // msg.ptr
> 0039:           mov     qword ptr -010h[RBP],7    // msg.length
> 0041:           lea     RCX,-010h[RBP]            // &msg[]
> 0045:           call      L0
> --- end of inserted assert failure code ---
> 004a:           mov     RSP,RBP
> 004d:           pop     RBP
> 004e:           ret
> -------------------------------------------
>
> 26 bytes of inserted Bloaty McBloatface code and 15 bytes of data. My proposal:
>
> _D4test4testFiZv:
> 0000:           push    RBP
> 0001:           mov     RBP,RSP
> 0004:           sub     RSP,040h
> 0008:           mov     010h[RBP],ECX
> 000b:           cmp     dword ptr 010h[RBP],0
> 000f:           jne     $+01h
> 0011:           hlt                                // look ma, 1 byte!
> 0012:           mov     RSP,RBP
> 0015:           pop     RBP
> 0016:           ret
>
> 1 byte of inserted code, and the data strings are gone as well.

I see you are concerned with the total size, which I understand. I think we misunderstood each other.

What I meant in terms of icache pollution is with the 'cold' is instead of generating:

if(!cond)
    _d_assert(__FILE__, __LINE__,message);
//rest of code

it should actually generate,

if (!cond)
    goto failed;
//rest of code

failed:
     _d_assert(__FILE__, __LINE__,message);//call is cold & out of line. no icache pollution

I'm not sure that it does that given the triviality of the example, but it looks like it doesn't.

December 01, 2017
On 11/30/2017 7:43 PM, Adam D. Ruppe wrote:
> I never, never use -release since it implies no bounds checking.

This is incorrect. -release leaves bounds checking on.


> BTW, are you against using `int 3` as the opcode instead? (0xCC)

That might be a better idea.
December 01, 2017
On 12/1/2017 2:57 AM, Walter Bright wrote:
> On 11/30/2017 7:43 PM, Adam D. Ruppe wrote:
>> I never, never use -release since it implies no bounds checking.
> 
> This is incorrect. -release leaves bounds checking on.

Correction:

https://dlang.org/dmd-windows.html#switch-release

"compile release version, which means not emitting run-time checks for contracts and asserts. Array bounds checking is not done for system and trusted functions, and assertion failures are undefined behaviour."
December 01, 2017
On Friday, 1 December 2017 at 11:01:13 UTC, Walter Bright wrote:
> assertion failures  are undefined behaviour."

=8-[