evilsong / gperftools

Automatically exported from code.google.com/p/gperftools
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Maybe macro for arm is wrong for revision r197 #510

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1.Checkout newest version r208.Since I'm trying to make gperftools working in 
my mips stb and did some changes on base/basictypes.h, I find the macro 
problem.  
2.See the code in r208.
#if defined(HAVE___ATTRIBUTE__)
# if (defined(__i386__) || defined(__x86_64__))
#   define CACHELINE_ALIGNED __attribute__((aligned(64)))
# elif defined(__arm__) //////////////////////////////////////this is for arm
#   define CACHELINE_ALIGNED __attribute__((aligned(32)))
# elif (defined(__PPC__) || defined(__PPC64__))
#   define CACHELINE_ALIGNED __attribute__((aligned(16)))
# elif (defined(__arm__)) ////////////////////////////////////this is also for 
arm
#   define CACHELINE_ALIGNED __attribute__((aligned(64)))
    // some ARMs have shorter cache lines (ARM1176JZF-S is 32 bytes for example) but obviously 64-byte aligned implies 32-byte aligned
# else
#   error Could not determine cache line length - unknown architecture
# endif
#else
# define CACHELINE_ALIGNED
#endif  // defined(HAVE___ATTRIBUTE__) && (__i386__ || __x86_64__)
3. There are two "# elif (defined(__arm__))" in the code, and the second was 
added in r197 to solve arm issue.
chappedm, is my pointing out of the problem right?

What is the expected output? What do you see instead?
Only one "# elif (defined(__arm__))" is needed.

What version of the product are you using? On what operating system?
gperftools r208 src.

Please provide any additional information below.

Original issue reported on code.google.com by xiaoyur...@gmail.com on 17 Mar 2013 at 4:28

GoogleCodeExporter commented 9 years ago
Indeed it seems wrong, shouldn't it have delete first __arm__ ifdef that 
defined 32-bytes alignment?

Original comment by zatr...@gmail.com on 19 Mar 2013 at 3:08

GoogleCodeExporter commented 9 years ago
Thanks for your reply.
"shouldn't it have delete first __arm__ ifdef that defined 32-bytes alignment?" 
Is the first arm define useful now?

Original comment by xiaoyur...@gmail.com on 26 Mar 2013 at 1:58

GoogleCodeExporter commented 9 years ago

Original comment by alkondratenko on 1 Apr 2013 at 5:15

GoogleCodeExporter commented 9 years ago
merged removal of first elif. It appears that newer arms indeed have 64 byte 
cachelines.

Original comment by alkondratenko on 1 Apr 2013 at 5:34