Skip to content
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

"Name Filtering" option is removing necessary ASM labels #406

Closed
scottmcm opened this issue Sep 17, 2018 · 3 comments
Closed

"Name Filtering" option is removing necessary ASM labels #406

scottmcm opened this issue Sep 17, 2018 · 3 comments
Labels
bug The playground isn't doing what it was intended to

Comments

@scottmcm
Copy link
Member

I don't know if this is exactly your problem, but godbolt's ASM for this example (https://rust.godbolt.org/z/_Wczb9) doesn't have the problem, so it might be.

Repro: https://play.rust-lang.org/?gist=f5e688bd60fb8de5018b31d4f05e7ab5&version=nightly&mode=release&edition=2015

.LBB6_9:
	lea	r14, [r13 - 1]
	lea	r15, [r12 + r14]
	lea	rdi, [r12 + r13]
	mov	ebx, 1
	mov	rsi, r15
	mov	rdx, rbx
	call	memcpy@PLT
	add	r13, rbx
	mov	rbx, r13
	sub	rbx, r14
	mov	rdx, rbp
	sub	rdx, r13
	lea	rdi, [r12 + r13]
	cmp	rdx, rbx
	ja	.LBB6_10
	mov	rsi, r15
	call	memcpy@PLT
	mov	rax, qword ptr [rsp]    # 8-byte Reload
	mov	qword ptr [rax + 16], rbp
	add	rsp, 8
	pop	rbx
	pop	r12
	pop	r13
	pop	r14
	pop	r15
	pop	rbp
	ret

Note the jump to LBB6_10, which is a label not in the output.

Turn off name filtering, and you get instead

.LBB6_9:
	lea	r14, [r13 - 1]
	lea	r15, [r12 + r14]
	lea	rdi, [r12 + r13]
	mov	ebx, 1
	.p2align	4, 0x90
.LBB6_10:                               # =>This Inner Loop Header: Depth=1
	mov	rsi, r15
	mov	rdx, rbx
	call	memcpy@PLT
	add	r13, rbx
	mov	rbx, r13
	sub	rbx, r14
	mov	rdx, rbp
	sub	rdx, r13
	lea	rdi, [r12 + r13]
	cmp	rdx, rbx
	ja	.LBB6_10
# %bb.11:
	mov	rsi, r15
	call	memcpy@PLT
	mov	rax, qword ptr [rsp]    # 8-byte Reload
	mov	qword ptr [rax + 16], rbp
	add	rsp, 8
	.cfi_def_cfa_offset 56
	pop	rbx
	.cfi_def_cfa_offset 48
	pop	r12
	.cfi_def_cfa_offset 40
	pop	r13
	.cfi_def_cfa_offset 32
	pop	r14
	.cfi_def_cfa_offset 24
	pop	r15
	.cfi_def_cfa_offset 16
	pop	rbp
	.cfi_def_cfa_offset 8
	ret

Which has the LBB6_10 label, as it should.

(Sorry for not reducing the repro.)

@shepmaster shepmaster added bug The playground isn't doing what it was intended to help wanted Not immediately going to be prioritized — ask for mentoring instructions! labels Sep 17, 2018
@shepmaster
Copy link
Member

/cc @Chandlore

@wfchandler
Copy link
Contributor

Thanks @scottmcm! Looks like this broke when LLVM comments assembly were added with #397 - the regex isn't expecting a '#' to be present in a line that declares a label.

I'll take a crack at it tonight.

@shepmaster
Copy link
Member

when LLVM comments assembly were added

Oops.

it me

wfchandler pushed a commit to wfchandler/rust-playground that referenced this issue Sep 18, 2018
The addition of LLVM comments to the generated
asm has prevented the filtering script from
recognizing some labels. This commit fixes
the missing labels and should keep comments
in the filtered asm.
@shepmaster shepmaster removed the help wanted Not immediately going to be prioritized — ask for mentoring instructions! label Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The playground isn't doing what it was intended to
Projects
None yet
Development

No branches or pull requests

3 participants