Can't force byte form of push instruction in clang-cl inline asm |
|||||||
Issue descriptionIn crrev.com/2701253005 we discussed improving IMMEDIATE_CRASH. msvc cl x86 supports something like #define CRASH() __asm int 3 __asm ud2 __asm push (__COUNTER__ % 256) however, in clang-cl (x64) the "% 256" part fails to compile. c:\src\cr\src\base\logging_unittest.cc(62,3): error: unexpected token in argument list CRASH(); ^ c:\src\cr\src\base\logging_unittest.cc(60,63): note: expanded from macro 'CRASH' #define CRASH() __asm int 3 __asm ud2 __asm push (__COUNTER__ % 256) ^ 1 error generated. "push byte ptr" doesn't help either. I could just use `push __COUNTER__` which would work until the 256'th CHECK, but since the point of this change is to reduce codesize, it'd be nice to make sure we got the immediate byte form. I couldn't think of another way to limit the range of the immediate in this syntax.
,
Feb 21 2017
I'm not sure where I'd put that? static_cast isn't valid asm syntax.
,
Feb 21 2017
Ah I though you coul "pass" argument to the as like in GCC:
asm volatile("int3; ud2; push %0;" ::"i"(static_cast<unsigned char>(__COUNTER__)))
Just realized that what you put after __asm is fed directly to the assembeler?
Maybe you can do some double macro hop? like
#define COUNTER_SHORT() (__COUNTER__ % 256)
#define CRASH() __asm ... COUNTER_SHORT()
hoping that the % 256 is computed before expanded?
,
Feb 21 2017
No such luch, it wants very literal textual instructions, without much/any interpretation.
,
Feb 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6a23306766d02a480c7992f91b51e9691d6511b8 commit 6a23306766d02a480c7992f91b51e9691d6511b8 Author: scottmg <scottmg@chromium.org> Date: Tue Feb 21 23:52:14 2017 Shorten IMMEDIATE_CRASH() code size on win clang R=thakis@chromium.org, primiano@chromium.org BUG= 664209 , 694670 Review-Url: https://codereview.chromium.org/2710703003 Cr-Commit-Position: refs/heads/master@{#451857} [modify] https://crrev.com/6a23306766d02a480c7992f91b51e9691d6511b8/base/logging.h
,
Mar 7 2017
http://llvm.org/viewvc/llvm-project?view=revision&revision=297057 adds some intrinsics you had asked for (maybe elsewhere). rnk, thoughts on comment 0?
,
Apr 10 2017
Actually +rnk. Thoughts on comment 0?
,
Apr 10 2017
re c#6, those intrinsics will still probably be tail-merged. We need some marker for "don't tail merge". re c#0, yeah, our Intel-style assembler isn't very good. I'd recommend GCC inline asm to make this work in the time being.
,
Oct 31 2017
Unable to triage this issue from TE-End, hence adding TE-NeedsTriageHelp label for further triage
,
Oct 31 2017
Turns out this was fixed, mostly by waiting. The LLVM Intel assembler just didn't understand modulo. Coby Tayree from Intel added support for the "MOD" spelling in June (https://reviews.llvm.org/D33876), and I extended it to recognize "%" this morning (r317011). Go open source! I guess it'll be in the next roll. :)
,
Oct 31 2017
,
Nov 7 2017
Clang has rolled, so I think this is fixed from a Clang point of view. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by primiano@chromium.org
, Feb 21 2017