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

Better test coverage for AggregateKind::RawPtr #124998

Closed
scottmcm opened this issue May 11, 2024 · 1 comment
Closed

Better test coverage for AggregateKind::RawPtr #124998

scottmcm opened this issue May 11, 2024 · 1 comment

Comments

@scottmcm
Copy link
Member

I noticed this today, so making this as a reminder, but if you see it and thing it sounds interesting, please pick it up!

Now that #123886 has landed, the general logic for aggregating immediates ought to work for pointers too, but seemingly it doesn't. We should add tests for whatever the problem is, so that it'll be easier for someone to validate a fix.

Steps:

  1. Delete this block that handles RawPtr specially:

mir::Rvalue::Aggregate(box mir::AggregateKind::RawPtr(..), ref fields) => {
let ty = rvalue.ty(self.mir, self.cx.tcx());
let layout = self.cx.layout_of(self.monomorphize(ty));
let [data, meta] = &*fields.raw else {
bug!("RawPtr fields: {fields:?}");
};
let data = self.codegen_operand(bx, data);
let meta = self.codegen_operand(bx, meta);
match (data.val, meta.val) {
(p @ OperandValue::Immediate(_), OperandValue::ZeroSized) => {
OperandRef { val: p, layout }
}
(OperandValue::Immediate(p), OperandValue::Immediate(m)) => {
OperandRef { val: OperandValue::Pair(p, m), layout }
}
_ => bug!("RawPtr operands {data:?} {meta:?}"),
}
}

  1. Build code to find an ICE. I'm not exactly what code it is, but it might be something related to crossbeam-utils. Might work to just ./x test --stage 2 tests/codegen, since it looks like it failed in the stage2 build in my other PR.

  2. Add minimal code to https://github.com/rust-lang/rust/blob/master/tests/codegen/mir-aggregate-no-alloca.rs that produces the same ICE when running ./x test --stage 1 tests/codegen.

  3. Undo the change from step 1 and add basic validation to the codegen test, similar to that done by the other tests in the file.

  4. Send a PR with the new tests.

  5. (Stretch goal) Delete the block mentioned in step 1 again and fix cg_ssa to pass the new test :D

@scottmcm scottmcm added A-codegen Area: Code generation E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. labels May 11, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 11, 2024
@scottmcm scottmcm removed E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. labels May 11, 2024
@scottmcm scottmcm self-assigned this May 11, 2024
@scottmcm
Copy link
Member Author

Oh, nvm, this isn't what I thought it was. I have a bug elsewhere.

@scottmcm scottmcm closed this as not planned Won't fix, can't repro, duplicate, stale May 11, 2024
@scottmcm scottmcm removed A-codegen Area: Code generation needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 11, 2024
@scottmcm scottmcm removed their assignment May 11, 2024
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

No branches or pull requests

2 participants