[PATCH] TargetInfo: Use DataLayout to compute pointer width in getPointerWidth()

[Alp Toker]

On 25/06/2014 23:31, Tom Stellard wrote:
> ---
>   include/clang/Basic/TargetInfo.h            |  4 +---
>   lib/Basic/TargetInfo.cpp                    |  9 +++++++++
>   test/CodeGen/record-lowering-non-zero-as.cl | 14 ++++++++++++++
>   3 files changed, 24 insertions(+), 3 deletions(-)
>   create mode 100644 test/CodeGen/record-lowering-non-zero-as.cl
>
> diff --git a/include/clang/Basic/TargetInfo.h b/include/clang/Basic/TargetInfo.h
> index e1d0116..65dc101 100644
> --- a/include/clang/Basic/TargetInfo.h
> +++ b/include/clang/Basic/TargetInfo.h
> @@ -790,9 +790,7 @@ public:
>     }
>   
>   protected:
> -  virtual uint64_t getPointerWidthV(unsigned AddrSpace) const {
> -    return PointerWidth;
> -  }
> +  virtual uint64_t getPointerWidthV(unsigned AddrSpace) const;
>     virtual uint64_t getPointerAlignV(unsigned AddrSpace) const {
>       return PointerAlign;
>     }
> diff --git a/lib/Basic/TargetInfo.cpp b/lib/Basic/TargetInfo.cpp
> index 71e39dd..d0f6306 100644
> --- a/lib/Basic/TargetInfo.cpp
> +++ b/lib/Basic/TargetInfo.cpp
> @@ -17,6 +17,7 @@
>   #include "clang/Basic/LangOptions.h"
>   #include "llvm/ADT/APFloat.h"
>   #include "llvm/ADT/STLExtras.h"
> +#include "llvm/IR/DataLayout.h"
>   #include "llvm/Support/ErrorHandling.h"
>   #include <cstdlib>
>   using namespace clang;
> @@ -242,6 +243,14 @@ bool TargetInfo::isTypeSigned(IntType T) {
>     };
>   }
>   
> +uint64_t TargetInfo::getPointerWidthV(unsigned AddrSpace) const {
> +  if (!DescriptionString)
> +    return PointerWidth;
> +
> +  llvm::DataLayout DL(DescriptionString);
> +  return DL.getPointerSizeInBits(AddrSpace);
> +}
> +

Hi Tom,

TargetInfo is meant to be independent of DataLayout IIRC, otherwise 
frontend functionality like semantic analysis would become dependent on 
backend targets.

...


>   /// setForcedLangOptions - Set forced language options.
>   /// Apply changes to the target information with respect to certain
>   /// language options which change the target configuration.
> diff --git a/test/CodeGen/record-lowering-non-zero-as.cl b/test/CodeGen/record-lowering-non-zero-as.cl
> new file mode 100644
> index 0000000..a96756f
> --- /dev/null
> +++ b/test/CodeGen/record-lowering-non-zero-as.cl
> @@ -0,0 +1,14 @@
> +// REQUIRES: r600-registered-target
> +// RUN: %clang -target r600 -mcpu=verde -S -emit-llvm -o - %s
> +
> +// Record lowering was crashing on SI and newer targets, because it
> +// was using the wrong size for test::ptr.  Since global memory
> +// has 64-bit pointers, sizeof(test::ptr) should be 8.

If the comment is accurate it indicates this should be a Sema test 
checking sizeof(test::ptr) with a static assert, rather than (or in 
addition to) a does-not-crash CodeGen test.

Alp.

> +
> +struct test {
> +  global int *ptr;
> +};
> +
> +void func(struct test t, global int *ptr) {
> +  *ptr = 0;
> +}

-- 
http://www.nuanti.com
the browser experts