Skip to content

[CASDB] OnDiskCAS should not create LeafNode when node has refs #4660

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

cachemeifyoucan
Copy link

Fix a bug that OnDiskCAS drops all the references when the data is
large and a leaf node is created. Leaf node should only be used when
there are no refs.

Fix a bug that OnDiskCAS drops all the references when the data is
large and a leaf node is created. Leaf node should only be used when
there are no refs.
@cachemeifyoucan
Copy link
Author

@swift-ci please test

@dexonsmith
Copy link

I suggest bumping the version (to v4?) since this one (v3?) will have bad data for anyone that saved something large. Otherwise LGTM.

@cachemeifyoucan
Copy link
Author

@swift-ci please test

@cachemeifyoucan cachemeifyoucan merged commit fc19121 into swiftlang:experimental/cas/main May 10, 2022
swift-ci pushed a commit that referenced this pull request Aug 12, 2025
M68k's SETCC instruction (`scc`) distinctly fills the destination byte
with all 1s. If boolean contents are set to `ZeroOrOneBooleanContent`,
LLVM can mistakenly think the destination holds `0x01` instead of `0xff`
and emit broken code as a result. This change corrects the boolean
content type to `ZeroOrNegativeOneBooleanContent`.

For example, this IR:

```llvm
define dso_local signext range(i8 0, 2) i8 @testBool(i32 noundef %a) local_unnamed_addr #0 {
entry:
  %cmp = icmp eq i32 %a, 4660
  %. = zext i1 %cmp to i8
  ret i8 %.
}
```

would previously build as:

```asm
testBool:                               ; @testBool
	cmpi.l	#4660, (4,%sp)
	seq	%d0
	and.l	#255, %d0
	rts
```

Notice the `zext` is erroneously not clearing the low bits, and thus the
register returns with 255 instead of 1. This patch fixes the issue:

```asm
testBool:                               ; @testBool
	cmpi.l	#4660, (4,%sp)
	seq	%d0
	and.l	#1, %d0
	rts
```

Most of the tests containing `scc` suffered from the same value error as
described above, so those tests have been updated to match the new
output (which also logically corrects them).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants