Incorrect Value when assigning list slices using OptimizationLevel::None #315

New Issue

derppening · 2023-09-05T16:33:08+08:00

derppening commented

2023-09-05 16:33:08 +08:00

Reproduction

class A:
	a: int32
	b: bool
	def __init__(self, a: int32, b: bool):
		self.a = a
		self.b = b

@extern
def output_int32_list(x: list[int32]):
	...

def run() -> int32:
	bl = [True, False]

	bl1 = bl[:]
	bl1[1:] = [True]
	output_int32_list([int32(b) for b in bl1])

	bl_1 = [True]
	bl2 = bl[:]
	bl2[1:2:2] = bl_1

	return 0

Python/NAC3 (OptimizationLevel::Default) Output:

[1, 1]

NAC3 (OptimizationLevel::None) Output:

[1, 255]

Changes which do not affect the bug

Changing the list assignment into a slice assignment for a single element

-	bl1[1:] = [True]
+	bl1[1:2] = [True]

which will result in

[1, 255]

Changing bl2[1:2:2] to bl2[1:2:<n>] where n > 1

Changes which stop the bug from manifesting

Direct assignment to bl instead of creating a new list and modifying it

-	bl1 = bl[:]
-	bl1[1:] = [True]
-	output_int32_list([int32(b) for b in bl1])
+	bl[1:] = [True]
+	output_int32_list([int32(b) for b in bl])

Decomposition of the list assignment into an element assignment + sublist

-	bl1[1:] = [True]
+	bl1[1] = True
+	bl1 = bl1[:2]

Changing bl2[1:2:2] to bl2[1:2:1]
Removing bl2[1:2:2] = bl_1

Other Notes

Although this does not affect NAC3 under default settings, it still appears to be a bug worth fixing as this may be a bug in codegen.

## Reproduction ```py class A: a: int32 b: bool def __init__(self, a: int32, b: bool): self.a = a self.b = b @extern def output_int32_list(x: list[int32]): ... def run() -> int32: bl = [True, False] bl1 = bl[:] bl1[1:] = [True] output_int32_list([int32(b) for b in bl1]) bl_1 = [True] bl2 = bl[:] bl2[1:2:2] = bl_1 return 0 ``` Python/NAC3 (`OptimizationLevel::Default`) Output: ``` [1, 1] ``` NAC3 (`OptimizationLevel::None`) Output: ``` [1, 255] ``` ### Changes which do not affect the bug - Changing the list assignment into a slice assignment for a single element ```diff - bl1[1:] = [True] + bl1[1:2] = [True] ``` which will result in ``` [1, 255] ``` - Changing `bl2[1:2:2]` to `bl2[1:2:<n>]` where `n > 1` ### Changes which stop the bug from manifesting - Direct assignment to `bl` instead of creating a new list and modifying it ```diff - bl1 = bl[:] - bl1[1:] = [True] - output_int32_list([int32(b) for b in bl1]) + bl[1:] = [True] + output_int32_list([int32(b) for b in bl]) ``` - Decomposition of the list assignment into an element assignment + sublist ```diff - bl1[1:] = [True] + bl1[1] = True + bl1 = bl1[:2] ``` - Changing `bl2[1:2:2]` to `bl2[1:2:1]` - Removing `bl2[1:2:2] = bl_1` ## Other Notes Although this does not affect NAC3 under default settings, it still appears to be a bug worth fixing as this may be a bug in codegen.

sb10q commented

2023-09-05 16:50:17 +08:00

Where exactly do you set -O?

derppening commented

2023-09-05 16:53:32 +08:00

nac3standalone/demo/src/codegen/mod.rs:236:

-            pass_builder.set_optimization_level(OptimizationLevel::Default);
+            pass_builder.set_optimization_level(OptimizationLevel::None);

I was doing that to debug some IR codegen.

`nac3standalone/demo/src/codegen/mod.rs:236`: ```diff - pass_builder.set_optimization_level(OptimizationLevel::Default); + pass_builder.set_optimization_level(OptimizationLevel::None); ``` I was doing that to debug some IR codegen.

sb10q commented

2023-09-05 16:54:54 +08:00

Okay. Why do you call that -O2/-O0?

Looks like probable broken IR indeed.

Okay. Why do you call that -O2/-O0? Looks like probable broken IR indeed.

derppening commented

2023-09-05 17:02:15 +08:00

Because according to LLVM, None/Less/Default/Aggressive corresponds to -O0/-O1/-O2/-O3 respectively. No worries, I will refer to them by the enum names from now on.

Because according to [LLVM](https://llvm.org/doxygen/CodeGen_8h_source.html), `None`/`Less`/`Default`/`Aggressive` corresponds to `-O0`/`-O1`/`-O2`/`-O3` respectively. No worries, I will refer to them by the enum names from now on.

derppening changed title from ~~Incorrect Value when assigning list slices using -O0~~ to Incorrect Value when assigning list slices using OptimizationLevel::None

2023-09-05 17:02:29 +08:00

derppening commented

2023-09-12 19:04:35 +08:00

This issue seemed to have gone away using the new pass manager. I guess more investigation is needed to determine whether this is a LLVM bug or NAC3 bug.

sb10q commented

2023-09-12 19:06:59 +08:00

Most likely a NAC3 bug (invalid IR emitted). I suggest tracking it down using the old pass manager code.

dpn commented

2023-09-12 23:45:44 +08:00

Yes; seems like this might potentially be a stack lifetime issue that is just masked by more aggressive optimisations.

For debugging IR issues, using the standalone LLVM tools to run the optimiser/codegen pipeline (opt, or even bugpoint for automation) can be very helpful.

Yes; seems like this might potentially be a stack lifetime issue that is just masked by more aggressive optimisations. For debugging IR issues, using the standalone LLVM tools to run the optimiser/codegen pipeline (`opt`, or even `bugpoint` for automation) can be very helpful.

derppening referenced this issue

2023-09-15 09:58:25 +08:00

Use LLVM New Pass Manager #319

derppening commented

2023-09-15 14:45:06 +08:00

So I found a much simpler way of reproducing the issue:

@extern
def output_int32_list(x: list[int32]):
	...

def run() -> int32:
	bl = [True, False]
	output_int32_list([int32(b) for b in bl])   # [1, 0]

	bl1 = bl[:]
	output_int32_list([int32(b) for b in bl1])  # [1, 0]

	bl1[1:] = [True]
	output_int32_list([int32(b) for b in bl1])  # [1, 255]
	output_int32_list([int32(b) for b in bl1])  # [1, 0]

	return 0

So I found a much simpler way of reproducing the issue: ```py @extern def output_int32_list(x: list[int32]): ... def run() -> int32: bl = [True, False] output_int32_list([int32(b) for b in bl]) # [1, 0] bl1 = bl[:] output_int32_list([int32(b) for b in bl1]) # [1, 0] bl1[1:] = [True] output_int32_list([int32(b) for b in bl1]) # [1, 255] output_int32_list([int32(b) for b in bl1]) # [1, 0] return 0 ```

sb10q referenced this issue

2023-09-15 16:47:58 +08:00

Use LLVM New Pass Manager #319

~~sb10q referenced this issue 2023-09-15 16:48:02 +08:00~~

Use LLVM New Pass Manager #319

~~sb10q referenced this issue 2023-09-15 16:48:03 +08:00~~

Use LLVM New Pass Manager #319

~~sb10q referenced this issue 2023-09-15 16:48:03 +08:00~~

Use LLVM New Pass Manager #319

derppening referenced this issue from a commit

2023-09-20 13:34:55 +08:00

core: Use i8 for boolean variable allocation