| Message ID | 20170303052459.2351-1-luc.vanoostenryck@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi Luc, On 3 March 2017 at 05:24, Luc Van Oostenryck <luc.vanoostenryck@gmail.com> wrote: > This should fox the problems of mixed types in casts. > Without much testing but should be essentialy OK. > > CC: Dibyendu Majumdar <mobile@majumdar.org.uk> > --- > sparse-llvm.c | 40 +++++++++++++++++++++++++++++++++++----- > 1 file changed, 35 insertions(+), 5 deletions(-) > > diff --git a/sparse-llvm.c b/sparse-llvm.c > index edd0615ec..92ad26845 100644 > --- a/sparse-llvm.c > +++ b/sparse-llvm.c > @@ -763,6 +763,8 @@ static void output_op_phi(struct function *fn, struct instruction *insn) > static void output_op_ptrcast(struct function *fn, struct instruction *insn) > { > LLVMValueRef src, target; > + LLVMTypeRef dtype; > + LLVMOpcode op; > char target_name[64]; > > src = insn->src->priv; > @@ -773,15 +775,31 @@ static void output_op_ptrcast(struct function *fn, struct instruction *insn) > > assert(!symbol_is_fp_type(insn->type)); > > - target = LLVMBuildBitCast(fn->builder, src, insn_symbol_type(fn->module, insn), target_name); > + dtype = insn_symbol_type(fn->module, insn); > + switch (LLVMGetTypeKind(LLVMTypeOf(src))) { > + case LLVMPointerTypeKind: > + op = LLVMBitCast; > + break; > + case LLVMIntegerTypeKind: > + op = LLVMIntToPtr; > + break; > + default: > + assert(0); > + } > > + target = LLVMBuildCast(fn->builder, op, src, dtype, target_name); > insn->target->priv = target; > } > > static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOpcode op) > { > LLVMValueRef src, target; > + LLVMTypeRef dtype; > char target_name[64]; > + unsigned int width; > + > + if (is_ptr_type(insn->type)) > + return output_op_ptrcast(fn, insn); > > src = insn->src->priv; > if (!src) > @@ -791,11 +809,23 @@ static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOp > > assert(!symbol_is_fp_type(insn->type)); > > - if (insn->size < LLVMGetIntTypeWidth(LLVMTypeOf(src))) > - target = LLVMBuildTrunc(fn->builder, src, insn_symbol_type(fn->module, insn), target_name); > - else > - target = LLVMBuildCast(fn->builder, op, src, insn_symbol_type(fn->module, insn), target_name); > + dtype = insn_symbol_type(fn->module, insn); > + switch (LLVMGetTypeKind(LLVMTypeOf(src))) { > + case LLVMPointerTypeKind: > + op = LLVMPtrToInt; > + break; > + case LLVMIntegerTypeKind: > + width = LLVMGetIntTypeWidth(LLVMTypeOf(src)); > + if (insn->size < width) > + op = LLVMTrunc; > + else if (insn->size == width) > + op = LLVMBitCast; > + break; > + default: > + assert(0); > + } > > + target = LLVMBuildCast(fn->builder, op, src, dtype, target_name); > insn->target->priv = target; > } > This doesn't quite work. The problem is that in op_cast, the pointer is being cast to int, but subsequent operations expect a pointer. Regards -- 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
On 3 March 2017 at 07:37, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote: > On 3 March 2017 at 05:24, Luc Van Oostenryck > <luc.vanoostenryck@gmail.com> wrote: >> This should fox the problems of mixed types in casts. >> Without much testing but should be essentialy OK. >> >> CC: Dibyendu Majumdar <mobile@majumdar.org.uk> >> --- >> sparse-llvm.c | 40 +++++++++++++++++++++++++++++++++++----- >> 1 file changed, 35 insertions(+), 5 deletions(-) >> >> diff --git a/sparse-llvm.c b/sparse-llvm.c >> index edd0615ec..92ad26845 100644 >> --- a/sparse-llvm.c >> +++ b/sparse-llvm.c >> @@ -763,6 +763,8 @@ static void output_op_phi(struct function *fn, struct instruction *insn) >> static void output_op_ptrcast(struct function *fn, struct instruction *insn) >> { >> LLVMValueRef src, target; >> + LLVMTypeRef dtype; >> + LLVMOpcode op; >> char target_name[64]; >> >> src = insn->src->priv; >> @@ -773,15 +775,31 @@ static void output_op_ptrcast(struct function *fn, struct instruction *insn) >> >> assert(!symbol_is_fp_type(insn->type)); >> >> - target = LLVMBuildBitCast(fn->builder, src, insn_symbol_type(fn->module, insn), target_name); >> + dtype = insn_symbol_type(fn->module, insn); >> + switch (LLVMGetTypeKind(LLVMTypeOf(src))) { >> + case LLVMPointerTypeKind: >> + op = LLVMBitCast; >> + break; >> + case LLVMIntegerTypeKind: >> + op = LLVMIntToPtr; >> + break; >> + default: >> + assert(0); >> + } >> >> + target = LLVMBuildCast(fn->builder, op, src, dtype, target_name); >> insn->target->priv = target; >> } >> >> static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOpcode op) >> { >> LLVMValueRef src, target; >> + LLVMTypeRef dtype; >> char target_name[64]; >> + unsigned int width; >> + >> + if (is_ptr_type(insn->type)) >> + return output_op_ptrcast(fn, insn); >> >> src = insn->src->priv; >> if (!src) >> @@ -791,11 +809,23 @@ static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOp >> >> assert(!symbol_is_fp_type(insn->type)); >> >> - if (insn->size < LLVMGetIntTypeWidth(LLVMTypeOf(src))) >> - target = LLVMBuildTrunc(fn->builder, src, insn_symbol_type(fn->module, insn), target_name); >> - else >> - target = LLVMBuildCast(fn->builder, op, src, insn_symbol_type(fn->module, insn), target_name); >> + dtype = insn_symbol_type(fn->module, insn); >> + switch (LLVMGetTypeKind(LLVMTypeOf(src))) { >> + case LLVMPointerTypeKind: >> + op = LLVMPtrToInt; >> + break; >> + case LLVMIntegerTypeKind: >> + width = LLVMGetIntTypeWidth(LLVMTypeOf(src)); >> + if (insn->size < width) >> + op = LLVMTrunc; >> + else if (insn->size == width) >> + op = LLVMBitCast; >> + break; >> + default: >> + assert(0); >> + } >> >> + target = LLVMBuildCast(fn->builder, op, src, dtype, target_name); >> insn->target->priv = target; >> } >> > > > This doesn't quite work. The problem is that in op_cast, the pointer > is being cast to int, but subsequent operations expect a pointer. > The problem occurs in this sequence: ptrcast.64 %r26 <- (64) %r20 add.64 %r27 <- %r26, %r23 cast.64 %r28 <- (64) %r27 store.64 %r28 -> 16[%arg1] The last cast finds that the instruction type in an integer and does a cast to Integer, so that causes the store to fail as it expects a pointer. Question in my mind is what is the result type of an add operation when one of the arguments is a pointer? Regards Dibyendu -- 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
Perhaps we need these fixes? https://patchwork.kernel.org/patch/8175541/ On 3 March 2017 at 18:06, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote: > On 3 March 2017 at 07:37, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote: >> On 3 March 2017 at 05:24, Luc Van Oostenryck >> <luc.vanoostenryck@gmail.com> wrote: >>> This should fox the problems of mixed types in casts. >>> Without much testing but should be essentialy OK. >>> >>> CC: Dibyendu Majumdar <mobile@majumdar.org.uk> >>> --- >>> sparse-llvm.c | 40 +++++++++++++++++++++++++++++++++++----- >>> 1 file changed, 35 insertions(+), 5 deletions(-) >>> >>> diff --git a/sparse-llvm.c b/sparse-llvm.c >>> index edd0615ec..92ad26845 100644 >>> --- a/sparse-llvm.c >>> +++ b/sparse-llvm.c >>> @@ -763,6 +763,8 @@ static void output_op_phi(struct function *fn, struct instruction *insn) >>> static void output_op_ptrcast(struct function *fn, struct instruction *insn) >>> { >>> LLVMValueRef src, target; >>> + LLVMTypeRef dtype; >>> + LLVMOpcode op; >>> char target_name[64]; >>> >>> src = insn->src->priv; >>> @@ -773,15 +775,31 @@ static void output_op_ptrcast(struct function *fn, struct instruction *insn) >>> >>> assert(!symbol_is_fp_type(insn->type)); >>> >>> - target = LLVMBuildBitCast(fn->builder, src, insn_symbol_type(fn->module, insn), target_name); >>> + dtype = insn_symbol_type(fn->module, insn); >>> + switch (LLVMGetTypeKind(LLVMTypeOf(src))) { >>> + case LLVMPointerTypeKind: >>> + op = LLVMBitCast; >>> + break; >>> + case LLVMIntegerTypeKind: >>> + op = LLVMIntToPtr; >>> + break; >>> + default: >>> + assert(0); >>> + } >>> >>> + target = LLVMBuildCast(fn->builder, op, src, dtype, target_name); >>> insn->target->priv = target; >>> } >>> >>> static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOpcode op) >>> { >>> LLVMValueRef src, target; >>> + LLVMTypeRef dtype; >>> char target_name[64]; >>> + unsigned int width; >>> + >>> + if (is_ptr_type(insn->type)) >>> + return output_op_ptrcast(fn, insn); >>> >>> src = insn->src->priv; >>> if (!src) >>> @@ -791,11 +809,23 @@ static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOp >>> >>> assert(!symbol_is_fp_type(insn->type)); >>> >>> - if (insn->size < LLVMGetIntTypeWidth(LLVMTypeOf(src))) >>> - target = LLVMBuildTrunc(fn->builder, src, insn_symbol_type(fn->module, insn), target_name); >>> - else >>> - target = LLVMBuildCast(fn->builder, op, src, insn_symbol_type(fn->module, insn), target_name); >>> + dtype = insn_symbol_type(fn->module, insn); >>> + switch (LLVMGetTypeKind(LLVMTypeOf(src))) { >>> + case LLVMPointerTypeKind: >>> + op = LLVMPtrToInt; >>> + break; >>> + case LLVMIntegerTypeKind: >>> + width = LLVMGetIntTypeWidth(LLVMTypeOf(src)); >>> + if (insn->size < width) >>> + op = LLVMTrunc; >>> + else if (insn->size == width) >>> + op = LLVMBitCast; >>> + break; >>> + default: >>> + assert(0); >>> + } >>> >>> + target = LLVMBuildCast(fn->builder, op, src, dtype, target_name); >>> insn->target->priv = target; >>> } >>> >> >> >> This doesn't quite work. The problem is that in op_cast, the pointer >> is being cast to int, but subsequent operations expect a pointer. >> > > The problem occurs in this sequence: > > ptrcast.64 %r26 <- (64) %r20 > add.64 %r27 <- %r26, %r23 > cast.64 %r28 <- (64) %r27 > store.64 %r28 -> 16[%arg1] > > The last cast finds that the instruction type in an integer and does a > cast to Integer, so that causes the store to fail as it expects a > pointer. > > Question in my mind is what is the result type of an add operation > when one of the arguments is a pointer? > > Regards > Dibyendu -- 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
On Fri, Mar 3, 2017 at 7:06 PM, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote: > The problem occurs in this sequence: > > ptrcast.64 %r26 <- (64) %r20 > add.64 %r27 <- %r26, %r23 > cast.64 %r28 <- (64) %r27 > store.64 %r28 -> 16[%arg1] > > The last cast finds that the instruction type in an integer and does a > cast to Integer, so that causes the store to fail as it expects a > pointer. What is the corresponding C code? With the patches I've posted, the three example you've given are handled without error for me. > Question in my mind is what is the result type of an add operation > when one of the arguments is a pointer? This should only occurs as some form of pointer arithmetic, so the result should be some kind of pointer. Luc -- 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
On 3 March 2017 at 19:50, Luc Van Oostenryck <luc.vanoostenryck@gmail.com> wrote: > On Fri, Mar 3, 2017 at 7:06 PM, Dibyendu Majumdar > <mobile@majumdar.org.uk> wrote: >> The problem occurs in this sequence: >> >> ptrcast.64 %r26 <- (64) %r20 >> add.64 %r27 <- %r26, %r23 >> cast.64 %r28 <- (64) %r27 >> store.64 %r28 -> 16[%arg1] >> >> The last cast finds that the instruction type in an integer and does a >> cast to Integer, so that causes the store to fail as it expects a >> pointer. > > What is the corresponding C code? > With the patches I've posted, the three example you've given are handled > without error for me. > C code below, it is the last += that is failing. typedef unsigned long long size_t; struct buffer_type_st { struct buffer_type_st *next_buffer; char *buffer; }; typedef struct buffer_type_st buffer_type_t; struct link_st { struct link_st *next; }; typedef struct link_st link_t; struct allocator_st { buffer_type_t *buffer_list; link_t *free_list; char *next_avail; char *last; size_t size; size_t n; }; typedef struct allocator_st allocator; extern void * alloc_node(allocator * a); extern void grow_allocator(allocator * a); void * alloc_node(allocator * a) { link_t *tmp; tmp = a->free_list; if (a->free_list) { a->free_list = (link_t *) (a->free_list->next); return (void *) tmp; } else { if (a->next_avail == a->last) { grow_allocator(a); } { void *tmp; tmp = (void *) a->next_avail; a->next_avail += a->size; return tmp; } } } -- 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
On Fri, Mar 3, 2017 at 7:30 PM, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote: > Perhaps we need these fixes? > > https://patchwork.kernel.org/patch/8175541/ I very much doubt it would make any difference. This patch is part of a serie enforcing correct handling of constant expression *as defined by the standard*. It won't change the type of an expression or something like this. Luc -- 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
On Sat, Mar 4, 2017 at 3:55 AM, Luc Van Oostenryck <luc.vanoostenryck@gmail.com> wrote: > On Fri, Mar 3, 2017 at 7:30 PM, Dibyendu Majumdar > <mobile@majumdar.org.uk> wrote: >> Perhaps we need these fixes? >> >> https://patchwork.kernel.org/patch/8175541/ > > I very much doubt it would make any difference. > This patch is part of a serie enforcing correct handling > of constant expression *as defined by the standard*. > It won't change the type of an expression or something like this. I second that. Those patch series give warnings but it should not affect the IR. Chris -- 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
diff --git a/sparse-llvm.c b/sparse-llvm.c index edd0615ec..92ad26845 100644 --- a/sparse-llvm.c +++ b/sparse-llvm.c @@ -763,6 +763,8 @@ static void output_op_phi(struct function *fn, struct instruction *insn) static void output_op_ptrcast(struct function *fn, struct instruction *insn) { LLVMValueRef src, target; + LLVMTypeRef dtype; + LLVMOpcode op; char target_name[64]; src = insn->src->priv; @@ -773,15 +775,31 @@ static void output_op_ptrcast(struct function *fn, struct instruction *insn) assert(!symbol_is_fp_type(insn->type)); - target = LLVMBuildBitCast(fn->builder, src, insn_symbol_type(fn->module, insn), target_name); + dtype = insn_symbol_type(fn->module, insn); + switch (LLVMGetTypeKind(LLVMTypeOf(src))) { + case LLVMPointerTypeKind: + op = LLVMBitCast; + break; + case LLVMIntegerTypeKind: + op = LLVMIntToPtr; + break; + default: + assert(0); + } + target = LLVMBuildCast(fn->builder, op, src, dtype, target_name); insn->target->priv = target; } static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOpcode op) { LLVMValueRef src, target; + LLVMTypeRef dtype; char target_name[64]; + unsigned int width; + + if (is_ptr_type(insn->type)) + return output_op_ptrcast(fn, insn); src = insn->src->priv; if (!src) @@ -791,11 +809,23 @@ static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOp assert(!symbol_is_fp_type(insn->type)); - if (insn->size < LLVMGetIntTypeWidth(LLVMTypeOf(src))) - target = LLVMBuildTrunc(fn->builder, src, insn_symbol_type(fn->module, insn), target_name); - else - target = LLVMBuildCast(fn->builder, op, src, insn_symbol_type(fn->module, insn), target_name); + dtype = insn_symbol_type(fn->module, insn); + switch (LLVMGetTypeKind(LLVMTypeOf(src))) { + case LLVMPointerTypeKind: + op = LLVMPtrToInt; + break; + case LLVMIntegerTypeKind: + width = LLVMGetIntTypeWidth(LLVMTypeOf(src)); + if (insn->size < width) + op = LLVMTrunc; + else if (insn->size == width) + op = LLVMBitCast; + break; + default: + assert(0); + } + target = LLVMBuildCast(fn->builder, op, src, dtype, target_name); insn->target->priv = target; }