[RFC/PATCH] sparse, llvm: Fix string globals access

[RFC/PATCH] sparse, llvm: Fix string globals access

[Pekka Enberg]

This patch attempts to fix code generation for global string access:

  static char *foo = "Foo !\n";

  extern int puts(const char *s);

  int main(int argc, char *argv[])
  {
          puts(foo);

          return 0;
  }

Unfortunately the generated executable SIGSEGVs:

  [penberg@tux sparse]$ ./sparsec foo.c && ./a.out
  Segmentation fault

Looking at the IR, Sparse/LLVM generates this:

  [penberg@tux sparse]$ ./sparse-llvm foo.c | llvm-dis
  ; ModuleID = '<stdin>'

  @"<noident>" = private global [7 x i8] c"Foo !\0A\00"
  @foo = private global [7 x i8]* @"<noident>"

  define i32 @main(i32, i8**) {
  L0:
    %load_target = load i64* bitcast ([7 x i8]* @"<noident>" to i64*)
    %2 = call i32 @puts(i64 %load_target)
    ret i32 0
  }

  declare i32 @puts(i64)

whereas Clang generates the following:

  @.str = private unnamed_addr constant [7 x i8] c"Foo !\0A\00", align 1

  define i32 @main(i32 %argc, i8** nocapture %argv) nounwind uwtable {
    %1 = tail call i32 @puts(i8* getelementptr inbounds ([7 x i8]* @.str, i64 0, i64 0)) nounwind
    ret i32 0
  }

  declare i32 @puts(i8* nocapture) nounwind

I'm not sure what the LLVM backend can do here. Sparse linearizes the code to
this which is why LLVM backend does the casting:

  [penberg@tux sparse]$ ./test-linearize foo.c
  main:
  .L0x7f341f6f1010:
          <entry-point>
          load.64     %r1 <- 0[foo]
          call.32     %r2 <- puts, %r1
          ret.32      $0

Comments?

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Jeff Garzik <jgarzik@redhat.com>
NOT-Signed-off-by: Pekka Enberg <penberg@kernel.org>
---
 sparse-llvm.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/sparse-llvm.c b/sparse-llvm.c
index 89c6a2e..6b94205 100644
--- a/sparse-llvm.c
+++ b/sparse-llvm.c
@@ -308,7 +308,6 @@ static LLVMValueRef pseudo_to_value(struct function *fn, struct instruction *ins
 		struct expression *expr;
 
 		assert(sym->bb_target == NULL);
-		assert(sym->ident == NULL);
 
 		expr = sym->initializer;
 		if (expr) {
@@ -326,6 +325,13 @@ static LLVMValueRef pseudo_to_value(struct function *fn, struct instruction *ins
 				result = LLVMConstGEP(data, indices, ARRAY_SIZE(indices));
 				break;
 			}
+			case EXPR_SYMBOL: {
+				struct symbol *sym = expr->symbol;
+
+				result = LLVMGetNamedGlobal(fn->module, show_ident(sym->ident));
+				assert(result != NULL);
+				break;
+			}
 			default:
 				assert(0);
 			}
-- 
1.7.7.6

Re: [RFC/PATCH] sparse, llvm: Fix string globals access

[Jeff Garzik]

On 06/08/2012 08:58 AM, Pekka Enberg wrote:
> This patch attempts to fix code generation for global string access:
>
>    static char *foo = "Foo !\n";
>
>    extern int puts(const char *s);
>
>    int main(int argc, char *argv[])
>    {
>            puts(foo);
>
>            return 0;
>    }
>
> Unfortunately the generated executable SIGSEGVs:
>
>    [penberg@tux sparse]$ ./sparsec foo.c&&  ./a.out
>    Segmentation fault
>
> Looking at the IR, Sparse/LLVM generates this:
>
>    [penberg@tux sparse]$ ./sparse-llvm foo.c | llvm-dis
>    ; ModuleID = '<stdin>'
>
>    @"<noident>" = private global [7 x i8] c"Foo !\0A\00"
>    @foo = private global [7 x i8]* @"<noident>"
>
>    define i32 @main(i32, i8**) {
>    L0:
>      %load_target = load i64* bitcast ([7 x i8]* @"<noident>" to i64*)
>      %2 = call i32 @puts(i64 %load_target)
>      ret i32 0
>    }
>
>    declare i32 @puts(i64)
>
> whereas Clang generates the following:
>
>    @.str = private unnamed_addr constant [7 x i8] c"Foo !\0A\00", align 1
>
>    define i32 @main(i32 %argc, i8** nocapture %argv) nounwind uwtable {
>      %1 = tail call i32 @puts(i8* getelementptr inbounds ([7 x i8]* @.str, i64 0, i64 0)) nounwind
>      ret i32 0
>    }
>
>    declare i32 @puts(i8* nocapture) nounwind
>
> I'm not sure what the LLVM backend can do here. Sparse linearizes the code to
> this which is why LLVM backend does the casting:
>
>    [penberg@tux sparse]$ ./test-linearize foo.c
>    main:
>    .L0x7f341f6f1010:
>            <entry-point>
>            load.64     %r1<- 0[foo]
>            call.32     %r2<- puts, %r1
>            ret.32      $0
>
> Comments?
>
> Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org>
> Cc: Christopher Li<sparse@chrisli.org>
> Cc: Jeff Garzik<jgarzik@redhat.com>
> NOT-Signed-off-by: Pekka Enberg<penberg@kernel.org>
> ---
>   sparse-llvm.c |    8 +++++++-
>   1 files changed, 7 insertions(+), 1 deletions(-)

ACK

Re: [RFC/PATCH] sparse, llvm: Fix string globals access

[Pekka Enberg]

On Fri, Jun 8, 2012 at 9:13 PM, Jeff Garzik <jeff@garzik.org> wrote:
> On 06/08/2012 08:58 AM, Pekka Enberg wrote:
>>
>> This patch attempts to fix code generation for global string access:
>>
>>   static char *foo = "Foo !\n";
>>
>>   extern int puts(const char *s);
>>
>>   int main(int argc, char *argv[])
>>   {
>>           puts(foo);
>>
>>           return 0;
>>   }
>>
>> Unfortunately the generated executable SIGSEGVs:
>>
>>   [penberg@tux sparse]$ ./sparsec foo.c&&  ./a.out
>>
>>   Segmentation fault
>>
>> Looking at the IR, Sparse/LLVM generates this:
>>
>>   [penberg@tux sparse]$ ./sparse-llvm foo.c | llvm-dis
>>   ; ModuleID = '<stdin>'
>>
>>   @"<noident>" = private global [7 x i8] c"Foo !\0A\00"
>>   @foo = private global [7 x i8]* @"<noident>"
>>
>>   define i32 @main(i32, i8**) {
>>   L0:
>>     %load_target = load i64* bitcast ([7 x i8]* @"<noident>" to i64*)
>>     %2 = call i32 @puts(i64 %load_target)
>>     ret i32 0
>>   }
>>
>>   declare i32 @puts(i64)
>>
>> whereas Clang generates the following:
>>
>>   @.str = private unnamed_addr constant [7 x i8] c"Foo !\0A\00", align 1
>>
>>   define i32 @main(i32 %argc, i8** nocapture %argv) nounwind uwtable {
>>     %1 = tail call i32 @puts(i8* getelementptr inbounds ([7 x i8]* @.str,
>> i64 0, i64 0)) nounwind
>>     ret i32 0
>>   }
>>
>>   declare i32 @puts(i8* nocapture) nounwind
>>
>> I'm not sure what the LLVM backend can do here. Sparse linearizes the code
>> to
>> this which is why LLVM backend does the casting:
>>
>>   [penberg@tux sparse]$ ./test-linearize foo.c
>>   main:
>>   .L0x7f341f6f1010:
>>           <entry-point>
>>           load.64     %r1<- 0[foo]
>>           call.32     %r2<- puts, %r1
>>           ret.32      $0
>>
>> Comments?
>>
>> Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org>
>> Cc: Christopher Li<sparse@chrisli.org>
>> Cc: Jeff Garzik<jgarzik@redhat.com>
>> NOT-Signed-off-by: Pekka Enberg<penberg@kernel.org>
>> ---
>>  sparse-llvm.c |    8 +++++++-
>>  1 files changed, 7 insertions(+), 1 deletions(-)
>
> ACK

Xi, so I guess your current work on linearize.c might fix the SIGSEGV
I'm seeing?
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] sparse, llvm: Fix string globals access

[Xi Wang]

On Jun 8, 2012, at 4:39 PM, Pekka Enberg wrote:
> 
> Xi, so I guess your current work on linearize.c might fix the SIGSEGV
> I'm seeing?

I guess the problem is that sparse-llvm generates an incorrect type
`load i64*' in llvm from the sparse instruction load.64.

	load.64     %r2 <- 0[foo]
	call.32     %r1 <- puts, %r2
	ret.32      $0

With the new ->ctype in pseudo sparse-llvm should be able to generate
the correct type.  I am playing with an LLVM backend with typed pseudos;
it generates the following code, which seems okay.

@0 = internal global [7 x i8] c"Foo !\0A\00", align 1
@foo = internal global i8* getelementptr inbounds ([7 x i8]* @0, i64 0, i64 0), align 8

define i32 @main(i32 %argc, i8** %argv) {
entry:
  %0 = load i8** @foo
  %1 = call i32 @puts(i8* %0)
  ret i32 0
}

- xi

Re: [RFC/PATCH] sparse, llvm: Fix string globals access

[Pekka Enberg]

On Sat, Jun 9, 2012 at 2:55 AM, Xi Wang <xi.wang@gmail.com> wrote:
> On Jun 8, 2012, at 4:39 PM, Pekka Enberg wrote:
>>
>> Xi, so I guess your current work on linearize.c might fix the SIGSEGV
>> I'm seeing?
>
> I guess the problem is that sparse-llvm generates an incorrect type
> `load i64*' in llvm from the sparse instruction load.64.
>
>        load.64     %r2 <- 0[foo]
>        call.32     %r1 <- puts, %r2
>        ret.32      $0
>
> With the new ->ctype in pseudo sparse-llvm should be able to generate
> the correct type.  I am playing with an LLVM backend with typed pseudos;
> it generates the following code, which seems okay.
>
> @0 = internal global [7 x i8] c"Foo !\0A\00", align 1
> @foo = internal global i8* getelementptr inbounds ([7 x i8]* @0, i64 0, i64 0), align 8
>
> define i32 @main(i32 %argc, i8** %argv) {
> entry:
>  %0 = load i8** @foo
>  %1 = call i32 @puts(i8* %0)
>  ret i32 0
> }

Right. Looking at your "splay" backend

static value_t emit_gep(builder_t builder, struct pseudo *src,
unsigned int offset, struct pseudo *dst)
{
	type_t charp = LLVMPointerType(LLVMInt8Type(), 0);
	value_t base = LLVMBuildPointerCast(builder, emit_pseudo(src), charp, "");
	value_t idx = LLVMConstInt(LLVMIntType(bits_in_pointer), offset, 0);
	value_t gep = LLVMBuildGEP(builder, base, &idx, 1, "");
	type_t type = LLVMPointerType(emit_type(dst->ctype), 0);

	return LLVMBuildPointerCast(builder, gep, type, "");
}

after which OP_LOAD translation becomes as simple as

	case OP_LOAD:
		return LLVMBuildLoad(builder, emit_gep(builder, insn->src,
insn->offset, insn->target), "");

Jeff, I guess we should do this for output_load() and output_store()
in sparse-llvm.c as well?
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] sparse, llvm: Fix string globals access

[Xi Wang]

On Jun 9, 2012, at 7:00 AM, Pekka Enberg wrote:
> 
> Right. Looking at your "splay" backend
> 
> static value_t emit_gep(builder_t builder, struct pseudo *src,
> unsigned int offset, struct pseudo *dst)
> {
> 	type_t charp = LLVMPointerType(LLVMInt8Type(), 0);
> 	value_t base = LLVMBuildPointerCast(builder, emit_pseudo(src), charp, "");
> 	value_t idx = LLVMConstInt(LLVMIntType(bits_in_pointer), offset, 0);
> 	value_t gep = LLVMBuildGEP(builder, base, &idx, 1, "");
> 	type_t type = LLVMPointerType(emit_type(dst->ctype), 0);
> 
> 	return LLVMBuildPointerCast(builder, gep, type, "");
> }
> 
> after which OP_LOAD translation becomes as simple as
> 
> 	case OP_LOAD:
> 		return LLVMBuildLoad(builder, emit_gep(builder, insn->src,
> insn->offset, insn->target), "");
> 
> Jeff, I guess we should do this for output_load() and output_store()
> in sparse-llvm.c as well?

Maybe we can do this for "pointer + integer" as well, using getelementptr
with offset computed by sparse (converting the pointer to char* first).
Unlike inttoptr/ptrtotint, this shouldn't disable LLVM's alias analysis.
I will give it a try later.

- xi

Re: [RFC/PATCH] sparse, llvm: Fix string globals access

[Jeff Garzik]

On 06/09/2012 07:00 AM, Pekka Enberg wrote:
> Jeff, I guess we should do this for output_load() and output_store()
> in sparse-llvm.c as well?


Yes, that is a definite improvement over the current hackery in 
output_op_load()