Details
Diff Detail
Event Timeline
lib/CodeGen/GlobalISel/LegalizerHelper.cpp | ||
---|---|---|
526 | Is this really needed? |
lib/CodeGen/GlobalISel/LegalizerHelper.cpp | ||
---|---|---|
526 | I’m not sure what you mean? Otherwise you’ll end up with an extload with the same source and dest size (which I think the verifier should reject) |
lib/CodeGen/GlobalISel/LegalizerHelper.cpp | ||
---|---|---|
526 | I agree the verifier should reject it. 's32 = G_ZEXTLOAD ... :: (load 4 from ...)' isn't exactly wrong but it serves no purpose compared to 's32 = G_LOAD ... :: (load 4 from ...)' | |
540 | However, this should be invalid for the same reason, 's32 = G_SEXT s32' doesn't make sense. It would be better for the case where it lowers to G_LOAD to just replace the opcode in place. If we have observers plumbed into the legalizer yet we should also call the changingInstr()+changedInstr() events. | |
lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp | ||
210 | This is redundant given that clampScalar() limits the type to exactly s32. It's nice for consistency but it costs a small amount of compile-time to perform the test. |
lib/CodeGen/GlobalISel/LegalizerHelper.cpp | ||
---|---|---|
540 | I don't see why this would actually happen? I suppose that depends on what the constraints are for the specified legalize actions. I don't think you should be allow to specify narrow on a type to the same type. Assuming narrowing a load to the same type isn't allowed, this shouldn't be an issue. |
LGTM
lib/CodeGen/GlobalISel/LegalizerHelper.cpp | ||
---|---|---|
540 | Sorry, I was thinking that if the code above selects the G_LOAD then we can end up with a G_[SZ]EXT that doesn't extend but that can only happen if the G_[SZ]EXTLOAD was invalid in the first place. |
Is this really needed?