sparse-llvm incorrect handling of function pointers

sparse-llvm incorrect handling of function pointers

[Dibyendu Majumdar]

Hi,

This example fails:

extern int (*f) (int);
int main(int argc, const char *argv[]) {
 if (f) {
  return (*f)(6);
 }
}

The linearized output is:

main:
.L0:
        <entry-point>
        load.64     %r1(f) <- 0[f]
        br          %r1(f), .L1, .L3
.L1:
        load        %r3 <- 0[%r1(f)]
        call.32     %r4 <- %r3, $6
        br          .L3
.L3:
        ret.32      %r4

It is the second load that is failing. Am investigating the cause - it
seems something to do with calc_memop_addr().

Regards
Dibyendu

Re: sparse-llvm incorrect handling of function pointers

[Luc Van Oostenryck]

On Fri, Mar 10, 2017 at 02:23:26PM +0000, Dibyendu Majumdar wrote:
> Hi,
> 
> This example fails:
> 
> extern int (*f) (int);
> int main(int argc, const char *argv[]) {
>  if (f) {
>   return (*f)(6);
>  }
> }
> 
> The linearized output is:
> 
> main:
> .L0:
>         <entry-point>
>         load.64     %r1(f) <- 0[f]
>         br          %r1(f), .L1, .L3
> .L1:
>         load        %r3 <- 0[%r1(f)]
>         call.32     %r4 <- %r3, $6
>         br          .L3
> .L3:
>         ret.32      %r4
> 
> It is the second load that is failing. Am investigating the cause - it
> seems something to do with calc_memop_addr().

No, it's a problem with the linearized code.
There is no reasons for this second load to even exist.
You can see this by replacing '(*f)(6)' by the equivalent 'f(6)'.

Also you should be careful with this example as there is no
return for the 'else' part which create some undefined value
which can create weirdness.

-- Luc Van Oostenryck

Re: sparse-llvm incorrect handling of function pointers

[Dibyendu Majumdar]

On 10 March 2017 at 17:44, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Fri, Mar 10, 2017 at 02:23:26PM +0000, Dibyendu Majumdar wrote:
>> Hi,
>>
>> This example fails:
>>
>> extern int (*f) (int);
>> int main(int argc, const char *argv[]) {
>>  if (f) {
>>   return (*f)(6);
>>  }
>> }
>>
>> The linearized output is:
>>
>> main:
>> .L0:
>>         <entry-point>
>>         load.64     %r1(f) <- 0[f]
>>         br          %r1(f), .L1, .L3
>> .L1:
>>         load        %r3 <- 0[%r1(f)]
>>         call.32     %r4 <- %r3, $6
>>         br          .L3
>> .L3:
>>         ret.32      %r4
>>
>> It is the second load that is failing. Am investigating the cause - it
>> seems something to do with calc_memop_addr().
>
> No, it's a problem with the linearized code.
> There is no reasons for this second load to even exist.
> You can see this by replacing '(*f)(6)' by the equivalent 'f(6)'.

Okay.

>
> Also you should be careful with this example as there is no
> return for the 'else' part which create some undefined value
> which can create weirdness.
>

Yes, here is an amended test.

static int testfunc(int i) {
 return i-6;
}
static int (*f) (int) = testfunc;
int main(int argc, const char *argv[]) {
 if (f) {
  return f(6);
 }
 else {
  return 1;
 }
}

I am creating a bunch of tests that can be run after compiling. The
current backend tests are not very useful as they do not actually run
or validate the results.

Regards
Dibyendu

Re: sparse-llvm incorrect handling of function pointers

[Luc Van Oostenryck]

On Fri, Mar 10, 2017 at 06:13:16PM +0000, Dibyendu Majumdar wrote:
> Yes, here is an amended test.
> 
> static int testfunc(int i) {
>  return i-6;
> }
> static int (*f) (int) = testfunc;
> int main(int argc, const char *argv[]) {
>  if (f) {
>   return f(6);
>  }
>  else {
>   return 1;
>  }
> }

This one is because of the static initialization.

> I am creating a bunch of tests that can be run after compiling.

Excellent, they are really needed.

> The current backend tests are not very useful as they do not actually run
> or validate the results.

Yes, indeed.

-- Luc Van Oostenryck

Re: sparse-llvm incorrect handling of function pointers

[Dibyendu Majumdar]

On 11 March 2017 at 11:54, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Fri, Mar 10, 2017 at 06:13:16PM +0000, Dibyendu Majumdar wrote:
>> Yes, here is an amended test.
>>
>> static int testfunc(int i) {
>>  return i-6;
>> }
>> static int (*f) (int) = testfunc;
>> int main(int argc, const char *argv[]) {
>>  if (f) {
>>   return f(6);
>>  }
>>  else {
>>   return 1;
>>  }
>> }
>
> This one is because of the static initialization.
>

Yes, this works now:

static int testfunc(int i) {
 return i-6;
}
/* initialiser does not work */
/* static int (*f) (int) = testfunc */
static int (*f) (int);
int main(int argc, const char *argv[]) {
 f = testfunc;
 if (f) {
  return f(6);
 }
 else {
  return 1;
 }
}

[PATCH] llvm: add script to display the deserialized LLVM IR

[Luc Van Oostenryck]

The goal is to use this in some of test cases, by comparing
the expected output with the actual one.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 sparse-llvm-dis | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100755 sparse-llvm-dis

diff --git a/sparse-llvm-dis b/sparse-llvm-dis
new file mode 100755
index 000000000..573f8bbd0
--- /dev/null
+++ b/sparse-llvm-dis
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+set +e
+
+DIRNAME=`dirname $0`
+DIS=`"${LLVM_CONFIG:-llvm-config}" --bindir`/llvm-dis
+
+if [ $# -eq 0 ]; then
+  echo "`basename $0`: no input files"
+  exit 1
+fi
+
+$DIRNAME/sparse-llvm $@ | $DIS
-- 
2.11.1

Re: [PATCH] llvm: add script to display the deserialized LLVM IR

[Christopher Li]

On Sat, Mar 11, 2017 at 7:58 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> The goal is to use this in some of test cases, by comparing
> the expected output with the actual one.
>
> diff --git a/sparse-llvm-dis b/sparse-llvm-dis
> new file mode 100755
> index 000000000..573f8bbd0
> --- /dev/null
> +++ b/sparse-llvm-dis
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +
> +set +e
> +
> +DIRNAME=`dirname $0`
> +DIS=`"${LLVM_CONFIG:-llvm-config}" --bindir`/llvm-dis
> +
> +if [ $# -eq 0 ]; then
> +  echo "`basename $0`: no input files"
> +  exit 1
> +fi
> +
> +$DIRNAME/sparse-llvm $@ | $DIS

Looks good. You might want to check llvm-dis and sparse-llvm is
there. I assume this script will not be invoked if llvm is not there
so it is not a big deal.

Chris

Re: [PATCH] llvm: add script to display the deserialized LLVM IR

[Luc Van Oostenryck]

On Tue, Apr 25, 2017 at 10:43 PM, Christopher Li <sparse@chrisli.org> wrote:
> On Sat, Mar 11, 2017 at 7:58 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> The goal is to use this in some of test cases, by comparing
>> the expected output with the actual one.
>>
>
> Looks good. You might want to check llvm-dis and sparse-llvm is
> there. I assume this script will not be invoked if llvm is not there
> so it is not a big deal.
>
> Chris

Yes, it need sparse-llvm anyway.

OTOH, there is no users for this script so it can kept on hold too.

-- Luc