0

I have a 1 line inline function which is part of a hotspot in my code. I would like to see if changing this to a macro would be beneficial. Writing as a function I did not have to worry about side effects. But how do I write a macro for this without side effects?

#define FLAG1_BIT 4
struct node
{
    unsigned long key;
    struct node* child[K];      //format <address,flag1,flag2,flag3>
};

static inline bool isFlag1Set(struct node* p)
{
    return ((uintptr_t) p & FLAG1_BIT) != 0;
}
5gon12eder
  • 24,280
  • 5
  • 45
  • 92
arunmoezhi
  • 3,082
  • 6
  • 35
  • 54
  • What side effects are you worried about? This code doesn't seem to be modifying anything. – murgatroid99 Jul 25 '14 at 21:27
  • The expression `((uintptr_t) p & FLAG1_BIT) != 0` does not have any side effects. – R Sahu Jul 25 '14 at 21:27
  • Probably I should have posted a function where I set the flag with `return (struct node*) ((uintptr_t) p | FLAG1_BIT);` – arunmoezhi Jul 25 '14 at 21:29
  • 2
    It's unlikely that a macro will give you better code than an inline function. And are you *really* using the third low-order bit of a pointer as a flag? – Keith Thompson Jul 25 '14 at 21:29
  • @KeithThompson: But if the compiler for some weird reasons fails to inline this code? – arunmoezhi Jul 25 '14 at 21:29
  • 1
    Does it fail? Have you looked at the generated code? – Keith Thompson Jul 25 '14 at 21:31
  • Small question: Is it UB to assume that `key` is at the first position of the struct? – dari Jul 25 '14 at 21:32
  • I used `-O3` flag with `gcc 4.6.3`. It inlined it. But for portability reasons I wanted to switch to a macro, so if someone else compiles it with a different compiler it should still work fine – arunmoezhi Jul 25 '14 at 21:32
  • @dari why can't the key be in the first position? – arunmoezhi Jul 25 '14 at 21:33
  • `struct node* ptr = arrayofNodes; while(isFlag1Set(ptr++) {...}` could have side effects when used as a macro. – James Curran Jul 25 '14 at 21:34
  • Clobbering a low-order bit of a pointer for use as a flag is extremely non-portable. I tried a quick example using your code, compiling with `gcc -m32`; for an array of `struct node`, every other element's address had that bit set. Can you guarantee that every `struct node` object in your program is 8-byte aligned? (Even if you can make that assumption, the code is still in principle not portable to systems with different addressing schemes.) – Keith Thompson Jul 25 '14 at 21:44
  • @KeithThompson: You are right. I'm making an assumption that `malloc` will always give me an address which is 8-byte aligned. And yes it is not portable. – arunmoezhi Jul 25 '14 at 21:46
  • The fact that you've identified this as a hotspot indicates that the compiler is not inlining it. Perhaps you forgot to turn on optimization before profiling the code? – Chris Dodd Jul 25 '14 at 23:59

2 Answers2

5

First, keep in mind, I can see no reason why the compiler wouldn't inline it, so there's a every good chance this exercise will accomplish nothing at all.

The first rule to avoiding side-effects is to ensure that the parameter appears only once in the definition. Next, in the definition, wrap the parameter and the whole definition in parenthesis:

#define isFlag1Set(p)   (((uintptr_t) (p) & FLAG1_BIT) != 0)
James Curran
  • 101,701
  • 37
  • 181
  • 258
-4

Try:

#define IS_FLAG_1_SET(p)                \
{                                       \
     (((uintptr_t) p & FLAG1_BIT) != 0) \
}
tehwallz
  • 134
  • 9